From 267280e5d889cb9a2a70352dcfbfd88c12df6748 Mon Sep 17 00:00:00 2001 From: "Andrew W. Harn" Date: Fri, 17 Jan 2025 16:04:00 -0500 Subject: [PATCH 01/21] Commit first set of changes (broken) Signed-off-by: Andrew W. Harn --- packages/imperative/src/censor/index.ts | 13 + packages/imperative/src/censor/src/Censor.ts | 255 ++++++++++++++++++ .../src/censor/src/doc/ICensorOptions.ts | 36 +++ .../__tests__/CommandProcessor.unit.test.ts | 3 +- .../src/cmd/src/CommandProcessor.ts | 60 ++--- .../src/cmd/src/response/CommandResponse.ts | 16 +- .../imperative/src/config/src/ProfileInfo.ts | 4 +- .../src/config/src/__mocks__/Config.ts | 7 +- packages/imperative/src/index.ts | 1 + .../logger/__tests__/LoggerUtils.unit.test.ts | 1 + packages/imperative/src/logger/src/Logger.ts | 14 +- .../imperative/src/logger/src/LoggerUtils.ts | 146 ++-------- .../__tests__/DeferredPromise.unit.test.ts | 20 +- .../imperative/src/utilities/src/CliUtils.ts | 9 + .../src/utilities/src/DeferredPromise.ts | 20 +- 15 files changed, 410 insertions(+), 195 deletions(-) create mode 100644 packages/imperative/src/censor/index.ts create mode 100644 packages/imperative/src/censor/src/Censor.ts create mode 100644 packages/imperative/src/censor/src/doc/ICensorOptions.ts diff --git a/packages/imperative/src/censor/index.ts b/packages/imperative/src/censor/index.ts new file mode 100644 index 0000000000..2e46bb1e33 --- /dev/null +++ b/packages/imperative/src/censor/index.ts @@ -0,0 +1,13 @@ +/* +* This program and the accompanying materials are made available under the terms of the +* Eclipse Public License v2.0 which accompanies this distribution, and is available at +* https://www.eclipse.org/legal/epl-v20.html +* +* SPDX-License-Identifier: EPL-2.0 +* +* Copyright Contributors to the Zowe Project. +* +*/ + +export * from "./src/Censor"; +export * from "./src/doc/ICensorOptions"; \ No newline at end of file diff --git a/packages/imperative/src/censor/src/Censor.ts b/packages/imperative/src/censor/src/Censor.ts new file mode 100644 index 0000000000..b4ad1b7f86 --- /dev/null +++ b/packages/imperative/src/censor/src/Censor.ts @@ -0,0 +1,255 @@ +/* +* This program and the accompanying materials are made available under the terms of the +* Eclipse Public License v2.0 which accompanies this distribution, and is available at +* https://www.eclipse.org/legal/epl-v20.html +* +* SPDX-License-Identifier: EPL-2.0 +* +* Copyright Contributors to the Zowe Project. +* +*/ + +import * as lodash from "lodash"; +import { Arguments } from "yargs"; +import { ICommandProfileProperty } from "../../cmd/src/doc/profiles/definition/ICommandProfileProperty"; +import { CliUtils } from "../../utilities/src/CliUtils"; +import { ICensorOptions } from "./doc/ICensorOptions"; +import { Config } from "../../config/src/Config"; +import { ImperativeConfig } from "../../utilities/src/ImperativeConfig"; +import { EnvironmentalVariableSettings } from "../../imperative/src/env/EnvironmentalVariableSettings"; +import { ICommandProfileTypeConfiguration } from "../../cmd"; +import { IProfileSchema, IProfileTypeConfiguration } from "../../profiles"; + +export class Censor { + /* + * NOTE(Kelosky): Ideally we might have a consolidated list for secure fields, but for now we'll just + * make sure they're collocated within the same class. + * + * NOTE(Harn): This list should be kept in sync with the base profile secure definitions and MUST be in camel case. + */ + private static readonly MAIN_CENSORED_OPTIONS = ["auth", "p", "pass", "password", "passphrase", "credentials", + "authentication", "basicAuth", "tv", "tokenValue", "certFilePassphrase"]; + + private static readonly MAIN_SECURE_PROMPT_OPTIONS = ["user", "password", "tokenValue", "passphrase"]; + + // The censor response. + public static readonly CENSOR_RESPONSE = "****"; + + // A set of default censored options. + public static get DEFAULT_CENSORED_OPTIONS(): string[] { + const censoredList = new Set(); + for (const option of this.MAIN_CENSORED_OPTIONS) { + censoredList.add(option); + censoredList.add(CliUtils.getOptionFormat(option).kebabCase); + } + return Array.from(censoredList); + } + + // Return a customized list of secure prompt options + public static get SECURE_PROMPT_OPTIONS(): string[] { + const censoredList = new Set(); + for (const option of this.MAIN_SECURE_PROMPT_OPTIONS) { + censoredList.add(option); + censoredList.add(CliUtils.getOptionFormat(option).kebabCase); + } + return Array.from(censoredList); + } + + // Set a censored options list that can be set and retrieved for each command. + private static censored_options: Set = new Set(this.DEFAULT_CENSORED_OPTIONS); + + // Keep a cached config object if provided in another function + private static mConfig: Config = null; + + // Return a customized list of censored options (or just the defaults if not set). + public static get CENSORED_OPTIONS(): string[] { + return Array.from(this.censored_options); + } + + /** + * Specifies whether a given property path (e.g. "profiles.lpar1.properties.host") is a special value or not. + * Special value: Refers to any value defined as secure in the schema definition. + * These values should be already masked by the application (and/or plugin) developer. + * @param prop Property path to determine if it is a special value + * @returns True - if the given property is to be treated as a special value; False - otherwise + */ + public static isSpecialValue(prop: string): boolean { + for (const v of this.CENSORED_OPTIONS) { + if (prop.endsWith(`.${v}`)) return true; + } + return false; + } + + /** + * Add a censored option, including it's camelCase and kebabCase versions + * @param {string} option - The option to censor + */ + public static addCensoredOption(option: string) { + this.censored_options.add(option); + this.censored_options.add(CliUtils.getOptionFormat(option).camelCase); + this.censored_options.add(CliUtils.getOptionFormat(option).kebabCase); + } + + /** + * Singleton implementation of an internal reference to the schema + */ + private static mSchema: ICommandProfileTypeConfiguration[] = null; + + /** + * Helper method to get an internal reference to the loaded profiles + */ + public static get profileSchemas(): ICommandProfileTypeConfiguration[] { + if (this.mSchema == null) this.mSchema = ImperativeConfig.instance.loadedConfig?.profiles ?? []; + return this.mSchema; + } + + /** + * Helper method to set an internal reference to loaded profiles + * @param _schemas - The schemas to pass in to set to the logger + */ + public static setProfileSchemas(_schemas: IProfileTypeConfiguration[] | Map) { + if (this.mSchema == null) { + this.mSchema = []; + } + if (_schemas instanceof Map) { + _schemas.forEach((v: IProfileSchema) => { + this.mSchema.push({ type: v.type, schema: v }); + }); + } else if (Array.isArray(_schemas)) { + _schemas.forEach((v: IProfileTypeConfiguration) => { + this.mSchema.push({ type: v.type, schema: v.schema }); + }); + } + } + + /** + * Generate and set the list of censored options. + * Attempt to source the censored options from the schema, config, and/or command being executed. + * @param {ICensorOptions} censorOpts - The objects to use to gather options that should be censored + */ + public static setCensoredOptions(censorOpts?: ICensorOptions) { + this.censored_options = new Set(this.DEFAULT_CENSORED_OPTIONS); + + if (censorOpts) { + // Save off the config object + this.mConfig = censorOpts.config; + + // If we have a ProfileTypeConfiguration (i.e. ImperativeConfig.instance.loadedConfig.profiles) + if (censorOpts.profiles) {this.setProfileSchemas(censorOpts.profiles);} + + for (const profileType of this.profileSchemas ?? []) { + for (const [propName, propValue] of Object.entries(profileType.schema.properties)) { + // Include the property itself + if (propValue.secure) { + this.addCensoredOption(propName); + } + + // Include any known aliases (if available) + if ((propValue as ICommandProfileProperty).optionDefinition?.aliases != null) { + for (const alias of (propValue as ICommandProfileProperty).optionDefinition.aliases) { + this.addCensoredOption(alias); + } + } + } + } + + // Include any secure options from the config + if (censorOpts.config) { + // Try to use the command and inputs to find the profiles being loaded + if (censorOpts.commandDefinition && censorOpts.commandArguments) { + const profiles = []; + for (const prof of censorOpts.commandDefinition.profile?.required || []) { + profiles.push(prof); + } + for (const prof of censorOpts.commandDefinition.profile?.optional || []) { + profiles.push(prof); + } + + for (const prof of profiles) { + // If the profile exists, append all of the secure props to the censored list + const profName = censorOpts.commandArguments?.[`${prof}-profile`]; + if (censorOpts.config.api.profiles.get(profName)) { + censorOpts.config.api.secure.securePropsForProfile(profName).forEach(prop => this.addCensoredOption(prop)); + } + } + } else { + // We only have a configuration file, assume every property that is secured should be censored + censorOpts.config.api.secure.findSecure(censorOpts.config.mProperties.profiles, "profiles").forEach( + prop => this.addCensoredOption(prop.split(".").pop()) + ); + } + } + } + } + + /** + * Copy and censor any sensitive CLI arguments before logging/printing + * @param {string[]} args - The args list to censor + * @returns {string[]} + */ + public static censorCLIArgs(args: string[]): string[] { + const newArgs: string[] = JSON.parse(JSON.stringify(args)); + const censoredValues = this.CENSORED_OPTIONS.map(CliUtils.getDashFormOfOption); + for (const value of censoredValues) { + if (args.indexOf(value) >= 0) { + const valueIndex = args.indexOf(value); + if (valueIndex < args.length - 1) { + newArgs[valueIndex + 1] = this.CENSOR_RESPONSE; // censor the argument after the option name + } + } + } + return newArgs; + } + + /** + * Copy and censor any sensitive CLI arguments before logging/printing + * @param {string} data - the data to censor + * @returns {string} - the censored data + */ + public static censorRawData(data: string, category: string = ""): string { + const config = this.mConfig ?? ImperativeConfig.instance?.config; + + // Return the data if not using config + if (!config?.exists) return data; + + // Return the data if we are printing to the console and masking is disabled + if (ImperativeConfig.instance?.envVariablePrefix) { + const envMaskOutput = EnvironmentalVariableSettings.read(ImperativeConfig.instance.envVariablePrefix).maskOutput.value; + // Hardcoding "console" instead of using Logger.DEFAULT_CONSOLE_NAME because of circular dependencies + if ((category === "console" || category === "json") && envMaskOutput.toUpperCase() === "FALSE") return data; + } + + let newData = data; + + const secureFields = config.api.secure.findSecure(config.mProperties.profiles, "profiles"); + for (const prop of secureFields) { + const sec = lodash.get(config.mProperties, prop); + if (sec && typeof sec !== "object" && !this.isSpecialValue(prop)) { + newData = newData.replace(new RegExp(sec, "gi"), this.CENSOR_RESPONSE); + } + } + return newData; + } + + /** + * Copy and censor a yargs argument object before logging + * @param {yargs.Arguments} args - the args to censor + * @returns {yargs.Arguments} - a censored copy of the arguments + */ + public static censorYargsArguments(args: Arguments): Arguments { + const newArgs: Arguments = JSON.parse(JSON.stringify(args)); + + for (const optionName of Object.keys(newArgs)) { + if (this.CENSORED_OPTIONS.indexOf(optionName) >= 0) { + const valueToCensor = newArgs[optionName]; + newArgs[optionName] = this.CENSOR_RESPONSE; + for (const checkAliasKey of Object.keys(newArgs)) { + if (newArgs[checkAliasKey] === valueToCensor) { + newArgs[checkAliasKey] = this.CENSOR_RESPONSE; + } + } + } + } + return newArgs; + } +} \ No newline at end of file diff --git a/packages/imperative/src/censor/src/doc/ICensorOptions.ts b/packages/imperative/src/censor/src/doc/ICensorOptions.ts new file mode 100644 index 0000000000..d2e63baac8 --- /dev/null +++ b/packages/imperative/src/censor/src/doc/ICensorOptions.ts @@ -0,0 +1,36 @@ +/* +* This program and the accompanying materials are made available under the terms of the +* Eclipse Public License v2.0 which accompanies this distribution, and is available at +* https://www.eclipse.org/legal/epl-v20.html +* +* SPDX-License-Identifier: EPL-2.0 +* +* Copyright Contributors to the Zowe Project. +* +*/ + +import { ICommandDefinition, ICommandArguments } from "../../../cmd"; +import { Config } from "../../../config"; +import { IProfileTypeConfiguration } from "../../../profiles"; + +export interface ICensorOptions { + /** + * An array of profile schema definitions + */ + profiles?: IProfileTypeConfiguration[]; + + /** + * The team config API + */ + config?: Config; + + /** + * The command definition for the command being executed + */ + commandDefinition?: ICommandDefinition; + + /** + * The command arguments for the command being executed + */ + commandArguments?: ICommandArguments; +} \ No newline at end of file diff --git a/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts b/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts index b48e7ab6b1..4898cb0049 100644 --- a/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts +++ b/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts @@ -191,6 +191,7 @@ const FAKE_HELP_GENERATOR: IHelpGenerator = { const ENV_VAR_PREFIX: string = "UNIT_TEST"; +/* eslint-disable deprecation/deprecation */ describe("Command Processor", () => { describe("Command Processor with --help and --version flags", () => { let faultyConfigProcessor: CommandProcessor; @@ -789,7 +790,7 @@ describe("Command Processor", () => { expect(commandResponse.error?.additionalDetails).toEqual("Syntax validation error!"); }); - it("should mask sensitive CLI options like user and password in log output", async () => { + fit("should mask sensitive CLI options like user and password in log output", async () => { // Allocate the command processor const processor: CommandProcessor = new CommandProcessor({ envVariablePrefix: ENV_VAR_PREFIX, diff --git a/packages/imperative/src/cmd/src/CommandProcessor.ts b/packages/imperative/src/cmd/src/CommandProcessor.ts index a3d0f077d0..a5b5f1537a 100644 --- a/packages/imperative/src/cmd/src/CommandProcessor.ts +++ b/packages/imperative/src/cmd/src/CommandProcessor.ts @@ -41,7 +41,8 @@ import { Config } from "../../config/src/Config"; import { ConfigUtils } from "../../config/src/ConfigUtils"; import { ConfigConstants } from "../../config/src/ConfigConstants"; import { IDaemonContext } from "../../imperative/src/doc/IDaemonContext"; -import { IHandlerResponseApi } from "../.."; +import { IHandlerResponseApi } from "./doc/response/api/handler/IHandlerResponseApi"; +import { Censor } from "../../censor/src/Censor"; /** @@ -357,41 +358,32 @@ export class CommandProcessor { `${CommandProcessor.ERROR_TAG} invoke(): Cannot invoke the handler for command "${this.definition.name}". The handler is blank.`); } - let commandLine = ImperativeConfig.instance.commandLine || this.commandLine; - - // determine if the command has the user option and mask the user value - let regEx = /--(user|u) ([^\s]+)/gi; - - if (commandLine.search(regEx) >= 0) { - commandLine = commandLine.replace(regEx, "--$1 ****"); - } - - // determine if the command has the password option and mask the password value - regEx = /--(password|pass|pw) ([^\s]+)/gi; - - if (commandLine.search(regEx) >= 0) { - commandLine = commandLine.replace(regEx, "--$1 ****"); - } - - // determine if the command has the token value option and mask the token value - regEx = /--(token-value|tokenValue|tv) ([^\s]+)/gi; - - if (commandLine.search(regEx) >= 0) { - commandLine = commandLine.replace(regEx, "--$1 ****"); - } - - // determine if the command has the cert key file option and mask the value - regEx = /--(cert-key-file|certKeyFile) ([^\s]+)/gi; - - if (commandLine.search(regEx) >= 0) { - commandLine = commandLine.replace(regEx, "--$1 ****"); - } + // Set up the censored options + Censor.setCensoredOptions({ + profiles: ImperativeConfig.instance.loadedConfig?.profiles, + config: ImperativeConfig.instance.config, + commandDefinition: this.definition, + commandArguments: params.arguments + }); - // determine if the command has the cert file passphrase option and mask the value - regEx = /--(cert-file-passphrase|certFilePassphrase) ([^\s]+)/gi; + let commandLine = ImperativeConfig.instance.commandLine || this.commandLine; - if (commandLine.search(regEx) >= 0) { - commandLine = commandLine.replace(regEx, "--$1 ****"); + for (const secureArg of Censor.CENSORED_OPTIONS) { + let regex: RegExp; + if (secureArg.length > 1) { + regex = new RegExp(`--${secureArg} ([^\\s]+)`, "gi"); + } else { + regex = new RegExp(`-${secureArg} ([^\\s]+)`, "gi"); + } + if (commandLine.search(regex) >= 0) { + if (secureArg.length > 1) { + commandLine = commandLine.replace(regex, `--${secureArg} ${Censor.CENSOR_RESPONSE}`); + } else { + commandLine = commandLine.replace(regex, `-${secureArg} ${Censor.CENSOR_RESPONSE}`); + } + } + console.dir(secureArg); + console.dir(commandLine); } // this.log.info(`post commandLine issued:\n\n${TextUtils.prettyJson(commandLine)}`); diff --git a/packages/imperative/src/cmd/src/response/CommandResponse.ts b/packages/imperative/src/cmd/src/response/CommandResponse.ts index 62c44037ef..5dfff67dcd 100644 --- a/packages/imperative/src/cmd/src/response/CommandResponse.ts +++ b/packages/imperative/src/cmd/src/response/CommandResponse.ts @@ -35,7 +35,7 @@ import { IPromptOptions } from "../doc/response/api/handler/IPromptOptions"; import { DaemonRequest } from "../../../utilities/src/DaemonRequest"; import { IDaemonResponse } from "../../../utilities/src/doc/IDaemonResponse"; import { Logger } from "../../../logger"; -import { LoggerUtils } from "../../../logger/src/LoggerUtils"; +import { Censor } from "../../../censor"; const DataObjectParser = require("dataobject-parser"); @@ -549,7 +549,7 @@ export class CommandResponse implements ICommandResponseApi { * @returns {string} - The formatted data or the original data.toString() if a buffer was passed */ public log(message: string | Buffer, ...values: any[]): string { - let msg: string = LoggerUtils.censorRawData(message.toString(), Logger.DEFAULT_CONSOLE_NAME); + let msg: string = Censor.censorRawData(message.toString(), Logger.DEFAULT_CONSOLE_NAME); if (!Buffer.isBuffer(message)) { msg = outer.formatMessage(msg.toString(), ...values) + "\n"; } @@ -565,7 +565,7 @@ export class CommandResponse implements ICommandResponseApi { * @returns {string} - The formatted data, or the original data.toString() if a buffer was passed */ public error(message: string | Buffer, ...values: any[]): string { - let msg: string = LoggerUtils.censorRawData(message.toString(), Logger.DEFAULT_CONSOLE_NAME); + let msg: string = Censor.censorRawData(message.toString(), Logger.DEFAULT_CONSOLE_NAME); if (!Buffer.isBuffer(message)) { msg = outer.formatMessage(msg.toString(), ...values) + "\n"; } @@ -581,7 +581,7 @@ export class CommandResponse implements ICommandResponseApi { * @returns {string} - The string that is printed (including the color codes) */ public errorHeader(message: string, delimeter = ":"): string { - let msg: string = LoggerUtils.censorRawData(message.toString(), Logger.DEFAULT_CONSOLE_NAME); + let msg: string = Censor.censorRawData(message.toString(), Logger.DEFAULT_CONSOLE_NAME); msg = TextUtils.chalk.red(msg + `${delimeter}\n`); outer.writeAndBufferStderr(msg); return msg; @@ -594,7 +594,7 @@ export class CommandResponse implements ICommandResponseApi { * @returns {Promise} */ public prompt(questionText: string, opts?: IPromptOptions): Promise { - const msg: string = LoggerUtils.censorRawData(questionText.toString(), Logger.DEFAULT_CONSOLE_NAME); + const msg: string = Censor.censorRawData(questionText.toString(), Logger.DEFAULT_CONSOLE_NAME); if (outer.mStream) { return new Promise((resolve) => { @@ -954,9 +954,9 @@ export class CommandResponse implements ICommandResponseApi { response = this.buildJsonResponse(); (response.stderr as any) = response.stderr.toString(); (response.stdout as any) = response.stdout.toString(); - response.message = LoggerUtils.censorRawData(response.message, "json"); - response.data = response.data ? JSON.parse(LoggerUtils.censorRawData(JSON.stringify(response.data), "json")) : undefined; - response.error = response.error ? JSON.parse(LoggerUtils.censorRawData(JSON.stringify(response.error), "json")) : undefined; + response.message = Censor.censorRawData(response.message, "json"); + response.data = response.data ? JSON.parse(Censor.censorRawData(JSON.stringify(response.data), "json")) : undefined; + response.error = response.error ? JSON.parse(Censor.censorRawData(JSON.stringify(response.error), "json")) : undefined; if (!this.mSilent) { this.writeStdout(JSON.stringify(response, null, 2)); diff --git a/packages/imperative/src/config/src/ProfileInfo.ts b/packages/imperative/src/config/src/ProfileInfo.ts index 978a34cd9b..e59c64ff25 100644 --- a/packages/imperative/src/config/src/ProfileInfo.ts +++ b/packages/imperative/src/config/src/ProfileInfo.ts @@ -38,7 +38,6 @@ import { IProfileLoaded, IProfileProperty, IProfileSchema } from "../../profiles import { CliUtils, ImperativeConfig } from "../../utilities"; import { ImperativeExpect } from "../../expect"; import { Logger } from "../../logger"; -import { LoggerUtils } from "../../logger/src/LoggerUtils"; import { IOptionsForAddConnProps, ISession, Session, SessConstants, ConnectionPropsForSessCfg } from "../../rest"; @@ -52,6 +51,7 @@ import { ConfigBuilder } from "./ConfigBuilder"; import { IAddProfTypeResult, IExtenderTypeInfo, IExtendersJsonOpts } from "./doc/IExtenderOpts"; import { IConfigLayer } from ".."; import { Constants } from "../../constants"; +import { Censor } from "../../censor"; /** * This class provides functions to retrieve profile-related information. @@ -1080,7 +1080,7 @@ export class ProfileInfo { } this.mHasValidSchema = lastSchema.path != null; - LoggerUtils.setProfileSchemas(this.mProfileSchemaCache); + Censor.setProfileSchemas(this.mProfileSchemaCache); } /** diff --git a/packages/imperative/src/config/src/__mocks__/Config.ts b/packages/imperative/src/config/src/__mocks__/Config.ts index a441bf88c8..10cf7661c5 100644 --- a/packages/imperative/src/config/src/__mocks__/Config.ts +++ b/packages/imperative/src/config/src/__mocks__/Config.ts @@ -22,10 +22,15 @@ export class Config { public get api() { return { - // profiles: new ConfigProfiles(this), + profiles: { + get: () => [] as any + }, plugins: { get: () => [] as any }, + secure: { + securePropsForProfile: () => [] as any + } // layers: new ConfigLayers(this), // secure: new ConfigSecure(this) }; diff --git a/packages/imperative/src/index.ts b/packages/imperative/src/index.ts index 4fe4232ed7..88e48c1bed 100644 --- a/packages/imperative/src/index.ts +++ b/packages/imperative/src/index.ts @@ -9,6 +9,7 @@ * */ +export * from "./censor"; export * from "./cmd"; export * from "./config"; export * from "./console"; diff --git a/packages/imperative/src/logger/__tests__/LoggerUtils.unit.test.ts b/packages/imperative/src/logger/__tests__/LoggerUtils.unit.test.ts index f1a6d8380f..e6b518628a 100644 --- a/packages/imperative/src/logger/__tests__/LoggerUtils.unit.test.ts +++ b/packages/imperative/src/logger/__tests__/LoggerUtils.unit.test.ts @@ -13,6 +13,7 @@ import { EnvironmentalVariableSettings } from "../../imperative/src/env/Environm import { LoggerUtils } from "../src/LoggerUtils"; import { ImperativeConfig } from "../../utilities/src/ImperativeConfig"; +/* eslint-disable deprecation/deprecation */ describe("LoggerUtils tests", () => { it("Should hide --password operand", () => { diff --git a/packages/imperative/src/logger/src/Logger.ts b/packages/imperative/src/logger/src/Logger.ts index b3034d2199..42582dfe3c 100644 --- a/packages/imperative/src/logger/src/Logger.ts +++ b/packages/imperative/src/logger/src/Logger.ts @@ -19,7 +19,7 @@ import { IConfigLogging } from "./doc/IConfigLogging"; import { LoggerManager } from "./LoggerManager"; import * as log4js from "log4js"; import { Console } from "../../console/src/Console"; -import { LoggerUtils } from "./LoggerUtils"; +import { Censor } from "../../censor"; /** * Note(Kelosky): it seems from the log4js doc that you only get a single @@ -194,7 +194,7 @@ export class Logger { * @returns {any} */ public debug(message: string, ...args: any[]): string { - const finalMessage = LoggerUtils.censorRawData(TextUtils.formatMessage.apply(this, [message].concat(args)), this.category); + const finalMessage = Censor.censorRawData(TextUtils.formatMessage.apply(this, [message].concat(args)), this.category); if (LoggerManager.instance.isLoggerInit || this.category === Logger.DEFAULT_CONSOLE_NAME) { this.logService.debug(this.getCallerFileAndLineTag() + finalMessage); } else { @@ -212,7 +212,7 @@ export class Logger { * @returns {any} */ public info(message: string, ...args: any[]): string { - const finalMessage = LoggerUtils.censorRawData(TextUtils.formatMessage.apply(this, [message].concat(args)), this.category); + const finalMessage = Censor.censorRawData(TextUtils.formatMessage.apply(this, [message].concat(args)), this.category); if (LoggerManager.instance.isLoggerInit || this.category === Logger.DEFAULT_CONSOLE_NAME) { this.logService.info(this.getCallerFileAndLineTag() + finalMessage); } else { @@ -230,7 +230,7 @@ export class Logger { * @returns {any} */ public warn(message: string, ...args: any[]): string { - const finalMessage = LoggerUtils.censorRawData(TextUtils.formatMessage.apply(this, [message].concat(args)), this.category); + const finalMessage = Censor.censorRawData(TextUtils.formatMessage.apply(this, [message].concat(args)), this.category); if (LoggerManager.instance.isLoggerInit || this.category === Logger.DEFAULT_CONSOLE_NAME) { this.logService.warn(this.getCallerFileAndLineTag() + finalMessage); } else { @@ -247,7 +247,7 @@ export class Logger { * @returns {any} */ public error(message: string, ...args: any[]): string { - const finalMessage = LoggerUtils.censorRawData(TextUtils.formatMessage.apply(this, [message].concat(args)), this.category); + const finalMessage = Censor.censorRawData(TextUtils.formatMessage.apply(this, [message].concat(args)), this.category); if (LoggerManager.instance.isLoggerInit || this.category === Logger.DEFAULT_CONSOLE_NAME) { this.logService.error(this.getCallerFileAndLineTag() + finalMessage); } else { @@ -264,7 +264,7 @@ export class Logger { * @returns {any} */ public fatal(message: string, ...args: any[]): string { - const finalMessage = LoggerUtils.censorRawData(TextUtils.formatMessage.apply(this, [message].concat(args)), this.category); + const finalMessage = Censor.censorRawData(TextUtils.formatMessage.apply(this, [message].concat(args)), this.category); if (LoggerManager.instance.isLoggerInit || this.category === Logger.DEFAULT_CONSOLE_NAME) { this.logService.fatal(this.getCallerFileAndLineTag() + finalMessage); } else { @@ -281,7 +281,7 @@ export class Logger { * @returns {any} */ public simple(message: string, ...args: any[]): string { - const finalMessage = LoggerUtils.censorRawData(TextUtils.formatMessage.apply(this, [message].concat(args)), this.category); + const finalMessage = Censor.censorRawData(TextUtils.formatMessage.apply(this, [message].concat(args)), this.category); if (LoggerManager.instance.isLoggerInit || this.category === Logger.DEFAULT_CONSOLE_NAME) { this.logService.info(finalMessage); } else { diff --git a/packages/imperative/src/logger/src/LoggerUtils.ts b/packages/imperative/src/logger/src/LoggerUtils.ts index 5a3465d9dc..290b48dccb 100644 --- a/packages/imperative/src/logger/src/LoggerUtils.ts +++ b/packages/imperative/src/logger/src/LoggerUtils.ts @@ -10,115 +10,58 @@ */ import { Arguments } from "yargs"; -import { EnvironmentalVariableSettings } from "../../imperative/src/env/EnvironmentalVariableSettings"; -import { CliUtils } from "../../utilities/src/CliUtils"; -import { ImperativeConfig } from "../../utilities/src/ImperativeConfig"; -import * as lodash from "lodash"; -import { Config } from "../../config/src/Config"; -import { IConfigLayer } from "../../config/src/doc/IConfigLayer"; import { ICommandProfileTypeConfiguration } from "../../cmd/src/doc/profiles/definition/ICommandProfileTypeConfiguration"; -import { ICommandProfileProperty } from "../../cmd/src/doc/profiles/definition/ICommandProfileProperty"; import { IProfileSchema } from "../../profiles/src/doc/definition/IProfileSchema"; +import { Censor } from "../../censor/src/Censor"; export class LoggerUtils { - public static readonly CENSOR_RESPONSE = "****"; + /** + * @deprecated Use Censor.CENSOR_RESPONSE + */ + public static CENSOR_RESPONSE = Censor.CENSOR_RESPONSE; /** - * NOTE(Kelosky): Ideally we might have a consolidated list for secure fields, but for now we'll just - * make sure they're collocated within the same class. + * @deprecated Use Censor.CENSORED_OPTIONS */ - public static CENSORED_OPTIONS = ["auth", "p", "pass", "password", "passphrase", "credentials", - "authentication", "basic-auth", "basicAuth", "tv", "token-value", "tokenValue", - "cert-file-passphrase", "certFilePassphrase"]; + public static CENSORED_OPTIONS = Censor.CENSORED_OPTIONS; - public static SECURE_PROMPT_OPTIONS = ["user", "password", "tokenValue", "passphrase"]; + /** + * @deprecated Use Censor.SECURE_PROMPT_OPTIONS + */ + public static SECURE_PROMPT_OPTIONS = Censor.SECURE_PROMPT_OPTIONS; /** * Copy and censor any sensitive CLI arguments before logging/printing * @param {string[]} args * @returns {string[]} + * @deprecated Use Censor.censorCLIArgs */ public static censorCLIArgs(args: string[]): string[] { - const newArgs: string[] = JSON.parse(JSON.stringify(args)); - const censoredValues = LoggerUtils.CENSORED_OPTIONS.map(CliUtils.getDashFormOfOption); - for (const value of censoredValues) { - if (args.indexOf(value) >= 0) { - const valueIndex = args.indexOf(value); - if (valueIndex < args.length - 1) { - newArgs[valueIndex + 1] = LoggerUtils.CENSOR_RESPONSE; // censor the argument after the option name - } - } - } - return newArgs; + return Censor.censorCLIArgs(args); } /** * Copy and censor a yargs argument object before logging * @param {yargs.Arguments} args the args to censor * @returns {yargs.Arguments} a censored copy of the arguments + * @deprecated Use Censor.censorYargsArguments */ public static censorYargsArguments(args: Arguments): Arguments { - const newArgs: Arguments = JSON.parse(JSON.stringify(args)); - - for (const optionName of Object.keys(newArgs)) { - if (LoggerUtils.CENSORED_OPTIONS.indexOf(optionName) >= 0) { - const valueToCensor = newArgs[optionName]; - newArgs[optionName] = LoggerUtils.CENSOR_RESPONSE; - for (const checkAliasKey of Object.keys(newArgs)) { - if (newArgs[checkAliasKey] === valueToCensor) { - newArgs[checkAliasKey] = LoggerUtils.CENSOR_RESPONSE; - } - } - } - } - return newArgs; - } - - /** - * Singleton implementation of an internal reference of ImperativeConfig.instance.config - */ - private static mConfig: Config = null; - private static get config(): Config { - if (LoggerUtils.mConfig == null) LoggerUtils.mConfig = ImperativeConfig.instance.config; - return LoggerUtils.mConfig; - } - - /** - * Singleton implementation of an internal reference to the active layer - * This should help with performance since one a single copy will be created for censoring data - */ - private static mLayer: IConfigLayer = null; - private static get layer(): IConfigLayer { - // if (LoggerUtils.mLayer == null) LoggerUtils.mLayer = LoggerUtils.config.api.layers.get(); - // Have to get a new copy every time because of how secure properties get lazy-loaded - LoggerUtils.mLayer = LoggerUtils.config.api.layers.get(); - return LoggerUtils.mLayer; + return Censor.censorYargsArguments(args); } /** - * Singleton implementation of an internal reference to the secure fields stored in the config + * @deprecated use Censor.profileSchemas */ - private static mSecureFields: string[] = null; - private static get secureFields(): string[] { - if (LoggerUtils.mSecureFields == null) LoggerUtils.mSecureFields = LoggerUtils.config.api.secure.secureFields(); - return LoggerUtils.mSecureFields; + public static get profileSchemas(): ICommandProfileTypeConfiguration[] { + return Censor.profileSchemas; } /** - * Singleton implementation of an internal reference to the loaded profiles + * @deprecated use Censor.setProfileSchemas */ - private static mProfiles: ICommandProfileTypeConfiguration[] = null; - public static get profileSchemas(): ICommandProfileTypeConfiguration[] { - if (LoggerUtils.mProfiles == null) LoggerUtils.mProfiles = ImperativeConfig.instance.loadedConfig?.profiles ?? []; - return LoggerUtils.mProfiles; - } public static setProfileSchemas(_schemas: Map) { - if (LoggerUtils.mProfiles == null) { - LoggerUtils.mProfiles = []; - } - _schemas.forEach((v: IProfileSchema) => { - LoggerUtils.mProfiles.push({ type: v.type, schema: v }); - }); + Censor.setProfileSchemas(_schemas); } /** @@ -127,60 +70,19 @@ export class LoggerUtils { * These values should be already masked by the application (and/or plugin) developer. * @param prop Property path to determine if it is a special value * @returns True - if the given property is to be treated as a special value; False - otherwise + * @deprecated Use Censor.isSpecialValue */ public static isSpecialValue = (prop: string): boolean => { - let specialValues = ["user", "password", "tokenValue", "keyPassphrase"]; - - /** - * Helper function that return a list of all optionDefinition names for a given ICommandProfileProperty - * @param prop profile property to get the optionDefinition names from - * @returns List of optionDefinition names - */ - const getPropertyNames = (prop: ICommandProfileProperty): string[] => { - const ret: string[] = []; - ret.push(prop.optionDefinition?.name); - prop.optionDefinitions?.map(opDef => ret.push(opDef.name)); - return ret; - }; - - for (const profile of LoggerUtils.profileSchemas) { - // eslint-disable-next-line unused-imports/no-unused-vars - for (const [_, prop] of Object.entries(profile.schema.properties)) { - if (prop.secure) specialValues = lodash.union(specialValues, getPropertyNames(prop)); - } - } - - // TODO: How to handle DNS resolution (using a wrong port) - // ex: zowe jobs list jobs --port 12345 - // May print the IP address of the given host if the resolved IP:port combination is not valid - - for (const v of specialValues) { - if (prop.endsWith(`.${v}`)) return true; - } - return false; + return Censor.isSpecialValue(prop); }; /** * Copy and censor any sensitive CLI arguments before logging/printing * @param {string} data * @returns {string} + * @deprecated Use Censor.censorRawData */ public static censorRawData(data: string, category: string = ""): string { - // Return the data if not using config - if (!ImperativeConfig.instance.config?.exists) return data; - - // Return the data if we are printing to the console and masking is disabled - const envMaskOutput = EnvironmentalVariableSettings.read(ImperativeConfig.instance.envVariablePrefix).maskOutput.value; - // Hardcoding "console" instead of using Logger.DEFAULT_CONSOLE_NAME because of circular dependencies - if ((category === "console" || category === "json") && envMaskOutput.toUpperCase() === "FALSE") return data; - - let newData = data; - const layer = LoggerUtils.layer; - for (const prop of LoggerUtils.secureFields) { - const sec = lodash.get(layer.properties, prop); - if (sec && typeof sec !== "object" && !LoggerUtils.isSpecialValue(prop)) - newData = newData.replace(new RegExp(sec, "gi"), LoggerUtils.CENSOR_RESPONSE); - } - return newData; + return Censor.censorRawData(data, category); } } diff --git a/packages/imperative/src/utilities/__tests__/DeferredPromise.unit.test.ts b/packages/imperative/src/utilities/__tests__/DeferredPromise.unit.test.ts index 146067c919..7ab1cf1bbe 100644 --- a/packages/imperative/src/utilities/__tests__/DeferredPromise.unit.test.ts +++ b/packages/imperative/src/utilities/__tests__/DeferredPromise.unit.test.ts @@ -1,13 +1,13 @@ -/** - * This program and the accompanying materials are made available under the terms of the - * Eclipse Public License v2.0 which accompanies this distribution, and is available at - * https://www.eclipse.org/legal/epl-v20.html - * - * SPDX-License-Identifier: EPL-2.0 - * - * Copyright Contributors to the Zowe Project. - * - */ +/* +* This program and the accompanying materials are made available under the terms of the +* Eclipse Public License v2.0 which accompanies this distribution, and is available at +* https://www.eclipse.org/legal/epl-v20.html +* +* SPDX-License-Identifier: EPL-2.0 +* +* Copyright Contributors to the Zowe Project. +* +*/ import { DeferredPromise, DeferredPromiseStatus } from "../src/DeferredPromise"; diff --git a/packages/imperative/src/utilities/src/CliUtils.ts b/packages/imperative/src/utilities/src/CliUtils.ts index 26d3096171..d9d7d7132e 100644 --- a/packages/imperative/src/utilities/src/CliUtils.ts +++ b/packages/imperative/src/utilities/src/CliUtils.ts @@ -32,6 +32,7 @@ export class CliUtils { * Used as the place holder when censoring arguments in messages/command output * @static * @memberof CliUtils + * @deprecated Use Censor.CENSOR_RESPONSE */ public static readonly CENSOR_RESPONSE = "****"; @@ -39,6 +40,7 @@ export class CliUtils { * A list of cli options/keywords that should normally be censored * @static * @memberof CliUtils + * @deprecated Use Censor.CENSORED_OPTIONS */ public static CENSORED_OPTIONS = ["auth", "p", "pass", "password", "passphrase", "credentials", "authentication", "basic-auth", "basicAuth"]; @@ -64,14 +66,17 @@ export class CliUtils { * Copy and censor any sensitive CLI arguments before logging/printing * @param {string[]} args - The args list to censor * @returns {string[]} + * @deprecated Use Censor.censorCLIArgs */ public static censorCLIArgs(args: string[]): string[] { const newArgs: string[] = JSON.parse(JSON.stringify(args)); + // eslint-disable-next-line deprecation/deprecation const censoredValues = CliUtils.CENSORED_OPTIONS.map(CliUtils.getDashFormOfOption); for (const value of censoredValues) { if (args.indexOf(value) >= 0) { const valueIndex = args.indexOf(value); if (valueIndex < args.length - 1) { + // eslint-disable-next-line deprecation/deprecation newArgs[valueIndex + 1] = CliUtils.CENSOR_RESPONSE; // censor the argument after the option name } } @@ -83,16 +88,20 @@ export class CliUtils { * Copy and censor a yargs argument object before logging * @param {yargs.Arguments} args the args to censor * @returns {yargs.Arguments} a censored copy of the arguments + * @deprecated Use Censor.censorYargsArguments */ public static censorYargsArguments(args: Arguments): Arguments { const newArgs: Arguments = JSON.parse(JSON.stringify(args)); for (const optionName of Object.keys(newArgs)) { + // eslint-disable-next-line deprecation/deprecation if (CliUtils.CENSORED_OPTIONS.indexOf(optionName) >= 0) { const valueToCensor = newArgs[optionName]; + // eslint-disable-next-line deprecation/deprecation newArgs[optionName] = CliUtils.CENSOR_RESPONSE; for (const checkAliasKey of Object.keys(newArgs)) { if (newArgs[checkAliasKey] === valueToCensor) { + // eslint-disable-next-line deprecation/deprecation newArgs[checkAliasKey] = CliUtils.CENSOR_RESPONSE; } } diff --git a/packages/imperative/src/utilities/src/DeferredPromise.ts b/packages/imperative/src/utilities/src/DeferredPromise.ts index 20b8fcfd22..65324d0855 100644 --- a/packages/imperative/src/utilities/src/DeferredPromise.ts +++ b/packages/imperative/src/utilities/src/DeferredPromise.ts @@ -1,13 +1,13 @@ -/** - * This program and the accompanying materials are made available under the terms of the - * Eclipse Public License v2.0 which accompanies this distribution, and is available at - * https://www.eclipse.org/legal/epl-v20.html - * - * SPDX-License-Identifier: EPL-2.0 - * - * Copyright Contributors to the Zowe Project. - * - */ +/* +* This program and the accompanying materials are made available under the terms of the +* Eclipse Public License v2.0 which accompanies this distribution, and is available at +* https://www.eclipse.org/legal/epl-v20.html +* +* SPDX-License-Identifier: EPL-2.0 +* +* Copyright Contributors to the Zowe Project. +* +*/ /* Status of the deferred promise */ export enum DeferredPromiseStatus { From e0e09295db0a19585d2874cd97d3d398fa4f9b2a Mon Sep 17 00:00:00 2001 From: "Andrew W. Harn" Date: Wed, 22 Jan 2025 10:55:12 -0500 Subject: [PATCH 02/21] Fix tests and logic Signed-off-by: Andrew W. Harn --- .../dsclp/TargetProfile.handler.unit.test.ts | 5 +-- packages/imperative/src/censor/src/Censor.ts | 23 +++++++++--- .../__tests__/CommandProcessor.unit.test.ts | 4 +-- .../src/cmd/src/CommandProcessor.ts | 2 -- .../src/config/src/__mocks__/Config.ts | 13 +++++-- .../logger/__tests__/LoggerUtils.unit.test.ts | 36 +++++++++++-------- .../src/session/ConnectionPropsForSessCfg.ts | 6 ++-- .../src/utilities/src/ImperativeConfig.ts | 6 +++- 8 files changed, 64 insertions(+), 31 deletions(-) diff --git a/packages/cli/__tests__/zosfiles/__unit__/copy/dsclp/TargetProfile.handler.unit.test.ts b/packages/cli/__tests__/zosfiles/__unit__/copy/dsclp/TargetProfile.handler.unit.test.ts index 9feccd3f0f..e7b0fc7877 100644 --- a/packages/cli/__tests__/zosfiles/__unit__/copy/dsclp/TargetProfile.handler.unit.test.ts +++ b/packages/cli/__tests__/zosfiles/__unit__/copy/dsclp/TargetProfile.handler.unit.test.ts @@ -50,9 +50,10 @@ describe("TargetProfileHandler", () => { api: { layers: { get: jest.fn() }, profiles: { get: getProfileMock }, - secure: { secureFields: jest.fn().mockReturnValue([]) } + secure: { findSecure: jest.fn().mockReturnValue([]), securePropsForProfile: jest.fn().mockReturnValue([]) } }, - exists: true + exists: true, + mProperties: { profiles: {} } }, envVariablePrefix: "ZOWE", loadedConfig: {} diff --git a/packages/imperative/src/censor/src/Censor.ts b/packages/imperative/src/censor/src/Censor.ts index b4ad1b7f86..01a9f894d2 100644 --- a/packages/imperative/src/censor/src/Censor.ts +++ b/packages/imperative/src/censor/src/Censor.ts @@ -27,10 +27,10 @@ export class Censor { * * NOTE(Harn): This list should be kept in sync with the base profile secure definitions and MUST be in camel case. */ - private static readonly MAIN_CENSORED_OPTIONS = ["auth", "p", "pass", "password", "passphrase", "credentials", + private static readonly MAIN_CENSORED_OPTIONS = ["auth", "pw", "pass", "password", "passphrase", "credentials", "authentication", "basicAuth", "tv", "tokenValue", "certFilePassphrase"]; - private static readonly MAIN_SECURE_PROMPT_OPTIONS = ["user", "password", "tokenValue", "passphrase"]; + private static readonly MAIN_SECURE_PROMPT_OPTIONS = ["user", "password", "tokenValue", "passphrase", "keyPassphrase"]; // The censor response. public static readonly CENSOR_RESPONSE = "****"; @@ -74,7 +74,22 @@ export class Censor { * @returns True - if the given property is to be treated as a special value; False - otherwise */ public static isSpecialValue(prop: string): boolean { - for (const v of this.CENSORED_OPTIONS) { + let specialValues = this.SECURE_PROMPT_OPTIONS; + const getPropertyNames = (prop: ICommandProfileProperty): string[] => { + const ret: string[] = []; + ret.push(prop.optionDefinition?.name); + prop.optionDefinitions?.map(opDef => ret.push(opDef.name)); + return ret; + }; + + for (const profile of this.profileSchemas) { + // eslint-disable-next-line unused-imports/no-unused-vars + for (const [_, prop] of Object.entries(profile.schema.properties)) { + if (prop.secure) specialValues = lodash.union(specialValues, getPropertyNames(prop)); + } + } + + for (const v of specialValues) { if (prop.endsWith(`.${v}`)) return true; } return false; @@ -168,7 +183,7 @@ export class Censor { for (const prof of profiles) { // If the profile exists, append all of the secure props to the censored list const profName = censorOpts.commandArguments?.[`${prof}-profile`]; - if (censorOpts.config.api.profiles.get(profName)) { + if (profName && censorOpts.config.api.profiles.get(profName)) { censorOpts.config.api.secure.securePropsForProfile(profName).forEach(prop => this.addCensoredOption(prop)); } } diff --git a/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts b/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts index 4898cb0049..2fea415179 100644 --- a/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts +++ b/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts @@ -790,7 +790,7 @@ describe("Command Processor", () => { expect(commandResponse.error?.additionalDetails).toEqual("Syntax validation error!"); }); - fit("should mask sensitive CLI options like user and password in log output", async () => { + it("should mask sensitive CLI options like user and password in log output", async () => { // Allocate the command processor const processor: CommandProcessor = new CommandProcessor({ envVariablePrefix: ENV_VAR_PREFIX, @@ -821,7 +821,7 @@ describe("Command Processor", () => { const commandResponse: ICommandResponse = await processor.invoke(parms); expect(mockLogInfo).toHaveBeenCalled(); - expect(logOutput).toContain("--user **** --password **** --token-value **** --cert-file-passphrase **** --cert-key-file ****"); + expect(logOutput).toContain("--user fakeUser --password **** --token-value **** --cert-file-passphrase **** --cert-key-file /fake/path"); }); it("should handle not being able to instantiate the handler", async () => { diff --git a/packages/imperative/src/cmd/src/CommandProcessor.ts b/packages/imperative/src/cmd/src/CommandProcessor.ts index a5b5f1537a..f7dd6ab998 100644 --- a/packages/imperative/src/cmd/src/CommandProcessor.ts +++ b/packages/imperative/src/cmd/src/CommandProcessor.ts @@ -382,8 +382,6 @@ export class CommandProcessor { commandLine = commandLine.replace(regex, `-${secureArg} ${Censor.CENSOR_RESPONSE}`); } } - console.dir(secureArg); - console.dir(commandLine); } // this.log.info(`post commandLine issued:\n\n${TextUtils.prettyJson(commandLine)}`); diff --git a/packages/imperative/src/config/src/__mocks__/Config.ts b/packages/imperative/src/config/src/__mocks__/Config.ts index 10cf7661c5..c26fed86eb 100644 --- a/packages/imperative/src/config/src/__mocks__/Config.ts +++ b/packages/imperative/src/config/src/__mocks__/Config.ts @@ -9,7 +9,7 @@ * */ -import { IConfigOpts, IConfigSchemaInfo } from "../.."; +import { IConfig, IConfigOpts, IConfigSchemaInfo } from "../.."; import { IConfigLayer } from "../../src/doc/IConfigLayer"; export class Config { @@ -29,7 +29,8 @@ export class Config { get: () => [] as any }, secure: { - securePropsForProfile: () => [] as any + securePropsForProfile: () => [] as any , + findSecure: () => [] as any } // layers: new ConfigLayers(this), // secure: new ConfigSecure(this) @@ -67,4 +68,12 @@ export class Config { original: "/some/path/to/schema.json" }; } + + public get mProperties() { + const config: IConfig = { + profiles: {}, + defaults: {} + } + return config; + } } \ No newline at end of file diff --git a/packages/imperative/src/logger/__tests__/LoggerUtils.unit.test.ts b/packages/imperative/src/logger/__tests__/LoggerUtils.unit.test.ts index e6b518628a..653cd50771 100644 --- a/packages/imperative/src/logger/__tests__/LoggerUtils.unit.test.ts +++ b/packages/imperative/src/logger/__tests__/LoggerUtils.unit.test.ts @@ -9,10 +9,17 @@ * */ +// Mock out config APIs +jest.mock("../../config/src/Config"); + import { EnvironmentalVariableSettings } from "../../imperative/src/env/EnvironmentalVariableSettings"; import { LoggerUtils } from "../src/LoggerUtils"; import { ImperativeConfig } from "../../utilities/src/ImperativeConfig"; +afterAll(() => { + jest.restoreAllMocks(); +}); + /* eslint-disable deprecation/deprecation */ describe("LoggerUtils tests", () => { @@ -36,8 +43,8 @@ describe("LoggerUtils tests", () => { expect(data).toContain(LoggerUtils.CENSOR_RESPONSE); }); - it("Should hide -p operand", () => { - const data = LoggerUtils.censorCLIArgs(["-p", "cantSeeMe"]); + it("Should hide --pw operand", () => { + const data = LoggerUtils.censorCLIArgs(["--pw", "cantSeeMe"]); expect(data).toContain(LoggerUtils.CENSOR_RESPONSE); }); @@ -60,22 +67,20 @@ describe("LoggerUtils tests", () => { const secrets = ["secret0", "secret1"]; let impConfigSpy: jest.SpyInstance = null; let envSettingsReadSpy: jest.SpyInstance = null; - let secureFields: jest.SpyInstance = null; - let layersGet: jest.SpyInstance = null; + let findSecure: jest.SpyInstance = null; let impConfig: any = null; // tried Partial but some properties complain about missing functionality beforeEach(() => { jest.restoreAllMocks(); - secureFields = jest.fn(); - layersGet = jest.fn(); + findSecure = jest.fn(); impConfigSpy = jest.spyOn(ImperativeConfig, "instance", "get"); envSettingsReadSpy = jest.spyOn(EnvironmentalVariableSettings, "read"); impConfig = { config: { exists: true, api: { - layers: { get: layersGet }, - secure: { secureFields } - } + secure: { findSecure } + }, + mProperties: {} } }; LoggerUtils.setProfileSchemas(new Map()); @@ -88,20 +93,22 @@ describe("LoggerUtils tests", () => { }); it("Console Output if the MASK_OUTPUT env var is FALSE", () => { - impConfigSpy.mockReturnValue({ config: { exists: true } }); + impConfigSpy.mockReturnValue({ config: { exists: true, api: { secure: { findSecure }}, mProperties: { profiles: []}}}); + findSecure.mockReturnValue([]); envSettingsReadSpy.mockReturnValue({ maskOutput: { value: "FALSE" } }); expect(LoggerUtils.censorRawData(secrets[1], "console")).toEqual(secrets[1]); }); describe("special value:", () => { beforeEach(() => { + impConfig.config.mProperties = {profiles: {secret: { properties: {}}}}; impConfigSpy.mockReturnValue(impConfig); envSettingsReadSpy.mockReturnValue({ maskOutput: { value: "TRUE" } }); }); const _lazyTest = (prop: string): [string, string] => { - secureFields.mockReturnValue([`secret.${prop}`, "secret.secret"]); - layersGet.mockReturnValue({ properties: { secret: { [prop]: secrets[0], secret: secrets[1] } } }); + findSecure.mockReturnValue([`profiles.secret.properties.${prop}`, "profiles.secret.properties.secret"]); + impConfig.config.mProperties.profiles.secret.properties = {[prop]: secrets[0], secret: secrets[1] }; const received = LoggerUtils.censorRawData(`visible secret: ${secrets[0]}, masked secret: ${secrets[1]}`); const expected = `visible secret: ${secrets[0]}, masked secret: ${LoggerUtils.CENSOR_RESPONSE}`; @@ -125,12 +132,13 @@ describe("LoggerUtils tests", () => { describe("should censor", () => { beforeEach(() => { + impConfig.config.mProperties = {profiles: {secret: { properties: {}}}}; impConfigSpy.mockReturnValue(impConfig); envSettingsReadSpy.mockReturnValue({ maskOutput: { value: "FALSE" } }); }); it("data if the logger category is not console, regardless of the MASK_OUTPUT env var value", () => { - secureFields.mockReturnValue(["secret.secret"]); - layersGet.mockReturnValue({ properties: { secret: { secret: secrets[1] } } }); + findSecure.mockReturnValue(["profiles.secret.properties.secret"]); + impConfig.config.mProperties.profiles.secret.properties = {secret: secrets[1] }; const received = LoggerUtils.censorRawData(`masked secret: ${secrets[1]}`, "This is not the console"); const expected = `masked secret: ${LoggerUtils.CENSOR_RESPONSE}`; expect(received).toEqual(expected); diff --git a/packages/imperative/src/rest/src/session/ConnectionPropsForSessCfg.ts b/packages/imperative/src/rest/src/session/ConnectionPropsForSessCfg.ts index db502ae1d7..6bcbb3b24f 100644 --- a/packages/imperative/src/rest/src/session/ConnectionPropsForSessCfg.ts +++ b/packages/imperative/src/rest/src/session/ConnectionPropsForSessCfg.ts @@ -20,6 +20,7 @@ import { ISession } from "./doc/ISession"; import { IProfileProperty } from "../../../profiles"; import { ConfigAutoStore } from "../../../config/src/ConfigAutoStore"; import { ConfigUtils } from "../../../config/src/ConfigUtils"; +import { Censor } from "../../../censor/src/Censor"; /** * Extend options for IPromptOptions for internal wrapper method @@ -353,11 +354,8 @@ export class ConnectionPropsForSessCfg { /** * List of properties on `sessCfg` object that should be kept secret and * may not appear in Imperative log files. - * - * NOTE(Kelosky): redundant from LoggerUtils.SECURE_PROMPT_OPTIONS - leaving - * for future date to consolidate */ - private static secureSessCfgProps: Set = new Set(["user", "password", "tokenValue", "passphrase"]); + private static secureSessCfgProps: Set = new Set(Censor.SECURE_PROMPT_OPTIONS); /** * List of prompt messages that is used when the CLI prompts for session diff --git a/packages/imperative/src/utilities/src/ImperativeConfig.ts b/packages/imperative/src/utilities/src/ImperativeConfig.ts index 7d49569448..3b6d15bc13 100644 --- a/packages/imperative/src/utilities/src/ImperativeConfig.ts +++ b/packages/imperative/src/utilities/src/ImperativeConfig.ts @@ -101,7 +101,11 @@ export class ImperativeConfig { * @returns {string} - the configured or default prefix for environmental variables for use in the environmental variable service */ public get envVariablePrefix(): string { - return this.loadedConfig.envVariablePrefix == null ? this.loadedConfig.name : this.loadedConfig.envVariablePrefix; + if (this.loadedConfig) { + return this.loadedConfig.envVariablePrefix == null ? this.loadedConfig.name : this.loadedConfig.envVariablePrefix; + } else { + return "ZOWE"; // Turns out this doesn't always return something, so set a default. + } } /** From 7c4ad2c78f2cb1afdfc17823b0e8aaeab871ab53 Mon Sep 17 00:00:00 2001 From: "Andrew W. Harn" Date: Wed, 22 Jan 2025 15:02:15 -0500 Subject: [PATCH 03/21] Make imports more specific Signed-off-by: Andrew W. Harn --- packages/imperative/src/censor/src/Censor.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/imperative/src/censor/src/Censor.ts b/packages/imperative/src/censor/src/Censor.ts index 01a9f894d2..19aca5483d 100644 --- a/packages/imperative/src/censor/src/Censor.ts +++ b/packages/imperative/src/censor/src/Censor.ts @@ -17,8 +17,9 @@ import { ICensorOptions } from "./doc/ICensorOptions"; import { Config } from "../../config/src/Config"; import { ImperativeConfig } from "../../utilities/src/ImperativeConfig"; import { EnvironmentalVariableSettings } from "../../imperative/src/env/EnvironmentalVariableSettings"; -import { ICommandProfileTypeConfiguration } from "../../cmd"; -import { IProfileSchema, IProfileTypeConfiguration } from "../../profiles"; +import { ICommandProfileTypeConfiguration } from "../../cmd/src/doc/profiles/definition/ICommandProfileTypeConfiguration"; +import { IProfileSchema} from "../../profiles/src/doc/definition/IProfileSchema"; +import { IProfileTypeConfiguration } from "../../profiles/src/doc/config/IProfileTypeConfiguration"; export class Censor { /* From 68fb52e7b700b07dba708112028104b05ac7e9dd Mon Sep 17 00:00:00 2001 From: "Andrew W. Harn" Date: Fri, 24 Jan 2025 15:47:48 -0500 Subject: [PATCH 04/21] Add tests Signed-off-by: Andrew W. Harn --- .../src/censor/__tests__/Censor.unit.test.ts | 279 ++++++++++++++++++ .../__snapshots__/Censor.unit.test.ts.snap | 34 +++ packages/imperative/src/censor/src/Censor.ts | 6 +- .../src/config/cmd/report-env/EnvQuery.ts | 5 +- .../imperative/src/logger/src/LoggerUtils.ts | 4 + 5 files changed, 323 insertions(+), 5 deletions(-) create mode 100644 packages/imperative/src/censor/__tests__/Censor.unit.test.ts create mode 100644 packages/imperative/src/censor/__tests__/__snapshots__/Censor.unit.test.ts.snap diff --git a/packages/imperative/src/censor/__tests__/Censor.unit.test.ts b/packages/imperative/src/censor/__tests__/Censor.unit.test.ts new file mode 100644 index 0000000000..603ad8f5e5 --- /dev/null +++ b/packages/imperative/src/censor/__tests__/Censor.unit.test.ts @@ -0,0 +1,279 @@ +/* +* This program and the accompanying materials are made available under the terms of the +* Eclipse Public License v2.0 which accompanies this distribution, and is available at +* https://www.eclipse.org/legal/epl-v20.html +* +* SPDX-License-Identifier: EPL-2.0 +* +* Copyright Contributors to the Zowe Project. +* +*/ + +// Mock out config APIs +jest.mock("../../config/src/Config"); + +import { EnvironmentalVariableSettings } from "../../imperative/src/env/EnvironmentalVariableSettings"; +import { Censor } from "../src/Censor"; +import { ImperativeConfig } from "../../utilities/src/ImperativeConfig"; + +beforeAll(() => { + Censor.setCensoredOptions(); +}); + +afterAll(() => { + jest.restoreAllMocks(); +}); + +describe("Censor tests", () => { + const nonCensoredOptionsList = ["certFile", "cert-file", "certKeyFile", "cert-key-file", "h", "host", "p", "port", "notSecret", "u", "user"]; + + it("should not change the default main censored options", () => { + expect(Censor.CENSORED_OPTIONS).toEqual(Censor.DEFAULT_CENSORED_OPTIONS); + expect(Censor.CENSORED_OPTIONS).toMatchSnapshot(); + }); + + it("should not change the default secure prompt options", () => { + expect(Censor.SECURE_PROMPT_OPTIONS).toMatchSnapshot(); + }); + + it("should not change the censor response", () => { + expect(Censor.CENSOR_RESPONSE).toMatchSnapshot(); + }); + + describe("addCensoredOption", () => { + beforeEach(() => { + Censor.setCensoredOptions(); + }); + + afterAll(() => { + Censor.setCensoredOptions(); + }); + + it("should add a censored option", () => { + Censor.addCensoredOption("secret"); + expect(Censor.CENSORED_OPTIONS).toContain("secret"); + }); + + it("should add a censored option in kebab and camel case 1", () => { + Censor.addCensoredOption("secret-value"); + expect(Censor.CENSORED_OPTIONS).toContain("secret-value"); + expect(Censor.CENSORED_OPTIONS).toContain("secretValue"); + }); + + it("should add a censored option in kebab and camel case 2", () => { + Censor.addCensoredOption("secretValue"); + expect(Censor.CENSORED_OPTIONS).toContain("secret-value"); + expect(Censor.CENSORED_OPTIONS).toContain("secretValue"); + }); + }); + + describe("censorCLIArgs", () => { + for (const opt of Censor.CENSORED_OPTIONS) { + it(`should hide --${opt} operand`, () => { + const data = Censor.censorCLIArgs([`--${opt}`, "cantSeeMe"]); + expect(data).toContain(Censor.CENSOR_RESPONSE); + }); + } + + for (const opt of nonCensoredOptionsList) { + it(`should not hide --${opt} operand`, () => { + const data = Censor.censorCLIArgs([`--${opt}`, "canSeeMe"]); + expect(data).not.toContain(Censor.CENSOR_RESPONSE); + }); + } + }); + + describe("censorRawData", () => { + const secrets = ["secret0", "secret1"]; + let impConfigSpy: jest.SpyInstance = null; + let envSettingsReadSpy: jest.SpyInstance = null; + let findSecure: jest.SpyInstance = null; + let impConfig: any = null; // tried Partial but some properties complain about missing functionality + beforeEach(() => { + jest.restoreAllMocks(); + findSecure = jest.fn(); + impConfigSpy = jest.spyOn(ImperativeConfig, "instance", "get"); + envSettingsReadSpy = jest.spyOn(EnvironmentalVariableSettings, "read"); + impConfig = { + config: { + exists: true, + api: { + secure: { findSecure } + }, + mProperties: {} + } + }; + Censor.setProfileSchemas(new Map()); + }); + + describe("should NOT censor", () => { + it("data if we are using old profiles 1", () => { + impConfigSpy.mockReturnValue({ config: { exists: false }, envVariablePrefix: "ZOWE" }); + expect(Censor.censorRawData(secrets[0])).toEqual(secrets[0]); + }); + + it("data if we are using old profiles 2", () => { + impConfigSpy.mockReturnValue({ config: undefined, envVariablePrefix: "ZOWE" }); + expect(Censor.censorRawData(secrets[0])).toEqual(secrets[0]); + }); + + it("Console Output if the MASK_OUTPUT env var is FALSE and category is console", () => { + impConfigSpy.mockReturnValue({ + config: { exists: true, api: { secure: { findSecure }}, mProperties: { profiles: []}}, + envVariablePrefix: "ZOWE" + }); + findSecure.mockReturnValue([]); + envSettingsReadSpy.mockReturnValue({ maskOutput: { value: "FALSE" } }); + expect(Censor.censorRawData(secrets[1], "console")).toEqual(secrets[1]); + }); + + it("Console Output if the MASK_OUTPUT env var is FALSE and category is json", () => { + impConfigSpy.mockReturnValue({ + config: { exists: true, api: { secure: { findSecure }}, mProperties: { profiles: []}}, + envVariablePrefix: "ZOWE" + }); + findSecure.mockReturnValue([]); + envSettingsReadSpy.mockReturnValue({ maskOutput: { value: "FALSE" } }); + expect(Censor.censorRawData(secrets[1], "json")).toEqual(secrets[1]); + }); + + describe("special value:", () => { + beforeEach(() => { + impConfig.config.mProperties = {profiles: {secret: { properties: {}}}}; + impConfigSpy.mockReturnValue(impConfig); + envSettingsReadSpy.mockReturnValue({ maskOutput: { value: "TRUE" } }); + }); + + const _lazyTest = (prop: string): [string, string] => { + findSecure.mockReturnValue([`profiles.secret.properties.${prop}`, "profiles.secret.properties.secret"]); + impConfig.config.mProperties.profiles.secret.properties = {[prop]: secrets[0], secret: secrets[1] }; + + const received = Censor.censorRawData(`visible secret: ${secrets[0]}, masked secret: ${secrets[1]}`); + const expected = `visible secret: ${secrets[0]}, masked secret: ${Censor.CENSOR_RESPONSE}`; + return [received, expected]; + }; + + for (const opt of Censor.SECURE_PROMPT_OPTIONS) { + it(`${opt}`, () => { + const [received, expected] = _lazyTest(opt); + expect(received).toEqual(expected); + }); + } + }); + }); + + describe("should censor", () => { + beforeEach(() => { + impConfig.config.mProperties = {profiles: {secret: { properties: {}}}}; + impConfigSpy.mockReturnValue(impConfig); + envSettingsReadSpy.mockReturnValue({ maskOutput: { value: "FALSE" } }); + }); + + afterAll(() => { + (Censor as any).mConfig = null; + }); + + it("data if the logger category is not console, regardless of the MASK_OUTPUT env var value 1", () => { + findSecure.mockReturnValue(["profiles.secret.properties.secret"]); + impConfig.config.mProperties.profiles.secret.properties = {secret: secrets[1] }; + const received = Censor.censorRawData(`masked secret: ${secrets[1]}`, "This is not the console"); + const expected = `masked secret: ${Censor.CENSOR_RESPONSE}`; + expect(received).toEqual(expected); + }); + + it("data if the logger category is not console, regardless of the MASK_OUTPUT env var value 2", () => { + (Censor as any).mConfig = { + exists: true, + mProperties: { + profiles: { + secret: { + properties: { + secret: secrets[1] + } + } + } + }, + api: { + secure: { + findSecure + } + } + }; + findSecure.mockReturnValue(["profiles.secret.properties.secret"]); + const received = Censor.censorRawData(`masked secret: ${secrets[1]}`, "This is not the console"); + const expected = `masked secret: ${Censor.CENSOR_RESPONSE}`; + expect(received).toEqual(expected); + }); + }); + }); + + describe("censorYargsArguments", () => { + for (const opt of Censor.CENSORED_OPTIONS) { + it(`should hide --${opt} operand`, () => { + const data = Censor.censorYargsArguments({ _: [], $0: "test", [opt]: "cantSeeMe" }); + expect(data[opt]).toContain(Censor.CENSOR_RESPONSE); + }); + } + + for (const opt of nonCensoredOptionsList) { + it(`should not hide --${opt} operand`, () => { + const data = Censor.censorYargsArguments({ _: [], $0: "test", [opt]: "canSeeMe" }); + expect(data[opt]).not.toContain(Censor.CENSOR_RESPONSE); + }); + } + + it("should handle passing in multiple arguments and censor the not normally censored options if they match", () => { + const args: any = {}; + args[Censor.CENSORED_OPTIONS[0]] = "cantSeeMe"; + for (const opt of nonCensoredOptionsList) { + args[opt] = "cantSeeMe"; + } + const data = Censor.censorYargsArguments({ _: [], $0: "test", ...args }); + expect(data[Censor.CENSORED_OPTIONS[0]]).toContain(Censor.CENSOR_RESPONSE); + for (const opt of nonCensoredOptionsList) { + expect(data[opt]).toContain(Censor.CENSOR_RESPONSE); + } + }); + }); + + describe("profileSchemas getter", () => { + let impConfigSpy: jest.SpyInstance = null; + + beforeEach(() => { + (Censor as any).mSchema = null; + impConfigSpy = jest.spyOn(ImperativeConfig, "instance", "get"); + }); + + afterAll(() => { + (Censor as any).mSchema = null; + }); + + it("should return the schema from ImperativeConfig", () => { + const mockedProfiles = [{ + type: "test", + schema: { + title: "Fake Profile Type", + description: "Fake Profile Description", + type: "object", + properties: { + test: { + type: "string", + description: "Fake Test Description" + } + } + } + }]; + impConfigSpy.mockReturnValue({ + loadedConfig: { + profiles: mockedProfiles + } + }); + expect(Censor.profileSchemas).toEqual(mockedProfiles); + }); + + it("should return nothing", () => { + impConfigSpy.mockReturnValue({}); + expect(Censor.profileSchemas).toEqual([]); + }); + }); +}); diff --git a/packages/imperative/src/censor/__tests__/__snapshots__/Censor.unit.test.ts.snap b/packages/imperative/src/censor/__tests__/__snapshots__/Censor.unit.test.ts.snap new file mode 100644 index 0000000000..1f2944bf3a --- /dev/null +++ b/packages/imperative/src/censor/__tests__/__snapshots__/Censor.unit.test.ts.snap @@ -0,0 +1,34 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Censor tests should not change the censor response 1`] = `"****"`; + +exports[`Censor tests should not change the default main censored options 1`] = ` +Array [ + "auth", + "authentication", + "basicAuth", + "basic-auth", + "certFilePassphrase", + "cert-file-passphrase", + "credentials", + "pw", + "pass", + "password", + "passphrase", + "tv", + "tokenValue", + "token-value", +] +`; + +exports[`Censor tests should not change the default secure prompt options 1`] = ` +Array [ + "keyPassphrase", + "key-passphrase", + "password", + "passphrase", + "tokenValue", + "token-value", + "user", +] +`; diff --git a/packages/imperative/src/censor/src/Censor.ts b/packages/imperative/src/censor/src/Censor.ts index 19aca5483d..f9701d0f39 100644 --- a/packages/imperative/src/censor/src/Censor.ts +++ b/packages/imperative/src/censor/src/Censor.ts @@ -28,10 +28,10 @@ export class Censor { * * NOTE(Harn): This list should be kept in sync with the base profile secure definitions and MUST be in camel case. */ - private static readonly MAIN_CENSORED_OPTIONS = ["auth", "pw", "pass", "password", "passphrase", "credentials", - "authentication", "basicAuth", "tv", "tokenValue", "certFilePassphrase"]; + private static readonly MAIN_CENSORED_OPTIONS = ["auth", "authentication", "basicAuth", "certFilePassphrase", "credentials", + "pw", "pass", "password", "passphrase", "tv", "tokenValue"]; - private static readonly MAIN_SECURE_PROMPT_OPTIONS = ["user", "password", "tokenValue", "passphrase", "keyPassphrase"]; + private static readonly MAIN_SECURE_PROMPT_OPTIONS = ["keyPassphrase", "password", "passphrase", "tokenValue", "user"]; // The censor response. public static readonly CENSOR_RESPONSE = "****"; diff --git a/packages/imperative/src/imperative/src/config/cmd/report-env/EnvQuery.ts b/packages/imperative/src/imperative/src/config/cmd/report-env/EnvQuery.ts index 70cc2a614c..bfde0fc1c8 100644 --- a/packages/imperative/src/imperative/src/config/cmd/report-env/EnvQuery.ts +++ b/packages/imperative/src/imperative/src/config/cmd/report-env/EnvQuery.ts @@ -25,6 +25,7 @@ import { IPluginJson } from "../../../plugins/doc/IPluginJson"; import { PluginIssues } from "../../../plugins/utilities/PluginIssues"; import { ItemId, IProbTest, probTests } from "./EnvItems"; +import { Censor } from "../../../../../censor"; /** * This interface represents the result from getEnvItemVal(). @@ -476,13 +477,13 @@ export class EnvQuery { private static getOtherZoweEnvVars(getResult: IGetItemVal): void { getResult.itemValMsg = ""; const envVars = process.env; + const secureCredsList = Censor.CENSORED_OPTIONS.map(opt => opt.toUpperCase().replaceAll("-", "_")); for (const nextVar of Object.keys(envVars)) { if (nextVar.startsWith("ZOWE_") && nextVar != "ZOWE_CLI_HOME" && nextVar != "ZOWE_APP_LOG_LEVEL" && nextVar != "ZOWE_IMPERATIVE_LOG_LEVEL") { getResult.itemValMsg += nextVar + " = " ; - if (nextVar.toUpperCase().includes("PASSWORD") || - nextVar.toUpperCase().includes("TOKEN")) + if (secureCredsList.includes(nextVar.toUpperCase())) { getResult.itemValMsg += "******"; } else { diff --git a/packages/imperative/src/logger/src/LoggerUtils.ts b/packages/imperative/src/logger/src/LoggerUtils.ts index 290b48dccb..6e27840429 100644 --- a/packages/imperative/src/logger/src/LoggerUtils.ts +++ b/packages/imperative/src/logger/src/LoggerUtils.ts @@ -14,6 +14,10 @@ import { ICommandProfileTypeConfiguration } from "../../cmd/src/doc/profiles/def import { IProfileSchema } from "../../profiles/src/doc/definition/IProfileSchema"; import { Censor } from "../../censor/src/Censor"; +/** + * @deprecated Use Censor + * Logging utilities + */ export class LoggerUtils { /** * @deprecated Use Censor.CENSOR_RESPONSE From aeb7e919c1053b4487a7722ddafd0f93b1d4a199 Mon Sep 17 00:00:00 2001 From: "Andrew W. Harn" Date: Thu, 30 Jan 2025 16:58:12 -0500 Subject: [PATCH 05/21] Add more tests, fix envQuery defect, handle multiple option definitions Signed-off-by: Andrew W. Harn --- .../src/censor/__tests__/Censor.unit.test.ts | 94 ++++++++++++++++++- packages/imperative/src/censor/src/Censor.ts | 38 +++++--- .../src/config/cmd/report-env/EnvQuery.ts | 4 +- 3 files changed, 119 insertions(+), 17 deletions(-) diff --git a/packages/imperative/src/censor/__tests__/Censor.unit.test.ts b/packages/imperative/src/censor/__tests__/Censor.unit.test.ts index 603ad8f5e5..170e063899 100644 --- a/packages/imperative/src/censor/__tests__/Censor.unit.test.ts +++ b/packages/imperative/src/censor/__tests__/Censor.unit.test.ts @@ -22,6 +22,7 @@ beforeAll(() => { afterAll(() => { jest.restoreAllMocks(); + Censor.setCensoredOptions(); }); describe("Censor tests", () => { @@ -258,7 +259,11 @@ describe("Censor tests", () => { properties: { test: { type: "string", - description: "Fake Test Description" + optionDefinition: { + name: "test", + type: "string", + description: "Fake Test Description" + } } } } @@ -276,4 +281,91 @@ describe("Censor tests", () => { expect(Censor.profileSchemas).toEqual([]); }); }); + + describe("isSpecialValues", () => { + let impConfigSpy: jest.SpyInstance = null; + + beforeEach(() => { + (Censor as any).mSchema = null; + impConfigSpy = jest.spyOn(ImperativeConfig, "instance", "get"); + }); + + afterAll(() => { + (Censor as any).mSchema = null; + }); + + describe("default list", () => { + for(const opt of Censor.SECURE_PROMPT_OPTIONS) { + it(`should return true for ${opt}`, () => { + expect(Censor.isSpecialValue("profiles.secret.properties." + opt)).toBe(true); + }); + } + + it("should return false for option 'test' when no profile schema is set", () => { + expect(Censor.isSpecialValue("profiles.test.properties.test")).toBe(false); + }); + + it("should return true for option 'test' when profile schema is set 1", () => { + const mockedProfiles = [{ + type: "test", + schema: { + title: "Fake Profile Type", + description: "Fake Profile Description", + type: "object", + properties: { + test: { + type: "string", + secure: true, + optionDefinition: { + name: "test", + type: "string", + description: "Fake Test Description", + } + } + } + } + }]; + impConfigSpy.mockReturnValue({ + loadedConfig: { + profiles: mockedProfiles + } + }); + expect(Censor.isSpecialValue("profiles.test.properties.test")).toBe(true); + }); + + it("should return true for option 'test' when profile schema is set 2", () => { + const mockedProfiles = [{ + type: "test", + schema: { + title: "Fake Profile Type", + description: "Fake Profile Description", + type: "object", + properties: { + test: { + type: "string", + secure: true, + optionDefinitions: [{ + name: "test1", + type: "string", + description: "Fake Test Description" + }, { + name: "test2", + type: "string", + description: "Fake Test Description" + }] + } + } + } + }]; + impConfigSpy.mockReturnValue({ + loadedConfig: { + profiles: mockedProfiles + } + }); + expect(Censor.isSpecialValue("profiles.test.properties.test")).toBe(false); + expect(Censor.isSpecialValue("profiles.test.properties.test1")).toBe(true); + expect(Censor.isSpecialValue("profiles.test.properties.test2")).toBe(true); + }); + }); + }); }); diff --git a/packages/imperative/src/censor/src/Censor.ts b/packages/imperative/src/censor/src/Censor.ts index f9701d0f39..ae28f75a53 100644 --- a/packages/imperative/src/censor/src/Censor.ts +++ b/packages/imperative/src/censor/src/Censor.ts @@ -101,9 +101,12 @@ export class Censor { * @param {string} option - The option to censor */ public static addCensoredOption(option: string) { - this.censored_options.add(option); - this.censored_options.add(CliUtils.getOptionFormat(option).camelCase); - this.censored_options.add(CliUtils.getOptionFormat(option).kebabCase); + // This option is required, but we do not want to ever allow null or undefined itself into the censored options + if (option != null) { + this.censored_options.add(option); + this.censored_options.add(CliUtils.getOptionFormat(option).camelCase); + this.censored_options.add(CliUtils.getOptionFormat(option).kebabCase); + } } /** @@ -154,17 +157,26 @@ export class Censor { if (censorOpts.profiles) {this.setProfileSchemas(censorOpts.profiles);} for (const profileType of this.profileSchemas ?? []) { - for (const [propName, propValue] of Object.entries(profileType.schema.properties)) { - // Include the property itself - if (propValue.secure) { - this.addCensoredOption(propName); - } - - // Include any known aliases (if available) - if ((propValue as ICommandProfileProperty).optionDefinition?.aliases != null) { - for (const alias of (propValue as ICommandProfileProperty).optionDefinition.aliases) { - this.addCensoredOption(alias); + for (const [_, prop] of Object.entries(profileType.schema.properties)) { + // Add censored options from the schema if the option is secure + if (prop.secure) { + // Handle the case of a single option definition + if (prop.optionDefinition) { + this.addCensoredOption(prop.optionDefinition.name); + for (const alias of prop.optionDefinition.aliases || []) { + // Remember to add the alias + this.addCensoredOption(alias); + } } + + // Handle the case of multiple option definitions + prop.optionDefinitions?.map(opDef => { + this.addCensoredOption(opDef.name); + for (const alias of opDef.aliases || []) { + // Remember to add the alias + this.addCensoredOption(alias); + } + }); } } } diff --git a/packages/imperative/src/imperative/src/config/cmd/report-env/EnvQuery.ts b/packages/imperative/src/imperative/src/config/cmd/report-env/EnvQuery.ts index bfde0fc1c8..6168196a73 100644 --- a/packages/imperative/src/imperative/src/config/cmd/report-env/EnvQuery.ts +++ b/packages/imperative/src/imperative/src/config/cmd/report-env/EnvQuery.ts @@ -483,12 +483,10 @@ export class EnvQuery { nextVar != "ZOWE_APP_LOG_LEVEL" && nextVar != "ZOWE_IMPERATIVE_LOG_LEVEL") { getResult.itemValMsg += nextVar + " = " ; - if (secureCredsList.includes(nextVar.toUpperCase())) - { + if (secureCredsList.some(secureOpt => nextVar.toUpperCase().includes(secureOpt))) { getResult.itemValMsg += "******"; } else { getResult.itemValMsg += envVars[nextVar]; - } getResult.itemValMsg += os.EOL; } From 7f172f67a422a0e113f4ff0254bed25e1b14bd3c Mon Sep 17 00:00:00 2001 From: "Andrew W. Harn" Date: Mon, 3 Feb 2025 10:31:58 -0500 Subject: [PATCH 06/21] Rearrange code a bit Signed-off-by: Andrew W. Harn --- .../src/censor/__tests__/Censor.unit.test.ts | 196 +++++++++++++----- packages/imperative/src/censor/src/Censor.ts | 164 +++++++++------ .../imperative/src/config/src/ProfileInfo.ts | 5 +- 3 files changed, 242 insertions(+), 123 deletions(-) diff --git a/packages/imperative/src/censor/__tests__/Censor.unit.test.ts b/packages/imperative/src/censor/__tests__/Censor.unit.test.ts index 170e063899..e80d6c7a21 100644 --- a/packages/imperative/src/censor/__tests__/Censor.unit.test.ts +++ b/packages/imperative/src/censor/__tests__/Censor.unit.test.ts @@ -51,18 +51,18 @@ describe("Censor tests", () => { }); it("should add a censored option", () => { - Censor.addCensoredOption("secret"); + (Censor as any).addCensoredOption("secret"); expect(Censor.CENSORED_OPTIONS).toContain("secret"); }); it("should add a censored option in kebab and camel case 1", () => { - Censor.addCensoredOption("secret-value"); + (Censor as any).addCensoredOption("secret-value"); expect(Censor.CENSORED_OPTIONS).toContain("secret-value"); expect(Censor.CENSORED_OPTIONS).toContain("secretValue"); }); it("should add a censored option in kebab and camel case 2", () => { - Censor.addCensoredOption("secretValue"); + (Censor as any).addCensoredOption("secretValue"); expect(Censor.CENSORED_OPTIONS).toContain("secret-value"); expect(Censor.CENSORED_OPTIONS).toContain("secretValue"); }); @@ -295,77 +295,159 @@ describe("Censor tests", () => { }); describe("default list", () => { + for(const opt of Censor.SECURE_PROMPT_OPTIONS) { it(`should return true for ${opt}`, () => { expect(Censor.isSpecialValue("profiles.secret.properties." + opt)).toBe(true); }); } - it("should return false for option 'test' when no profile schema is set", () => { - expect(Censor.isSpecialValue("profiles.test.properties.test")).toBe(false); - }); + for(const opt of Censor.CENSORED_OPTIONS) { + if (!Censor.SECURE_PROMPT_OPTIONS.includes(opt)) { + it(`should return false for ${opt}`, () => { + expect(Censor.isSpecialValue("profiles.secret.properties." + opt)).toBe(false); + }); + } + } + }); - it("should return true for option 'test' when profile schema is set 1", () => { - const mockedProfiles = [{ - type: "test", - schema: { - title: "Fake Profile Type", - description: "Fake Profile Description", - type: "object", - properties: { - test: { + it("should return false for option 'test' when no profile schema is set", () => { + expect(Censor.isSpecialValue("profiles.test.properties.test")).toBe(false); + }); + + it("should return true for option 'test' when profile schema is set 1", () => { + const mockedProfiles = [{ + type: "test", + schema: { + title: "Fake Profile Type", + description: "Fake Profile Description", + type: "object", + properties: { + test: { + type: "string", + secure: true, + optionDefinition: { + name: "test", type: "string", - secure: true, - optionDefinition: { - name: "test", - type: "string", - description: "Fake Test Description", - } + description: "Fake Test Description", } } } - }]; - impConfigSpy.mockReturnValue({ - loadedConfig: { - profiles: mockedProfiles - } - }); - expect(Censor.isSpecialValue("profiles.test.properties.test")).toBe(true); + } + }]; + impConfigSpy.mockReturnValue({ + loadedConfig: { + profiles: mockedProfiles + } }); + expect(Censor.isSpecialValue("profiles.test.properties.test")).toBe(true); + }); - it("should return true for option 'test' when profile schema is set 2", () => { - const mockedProfiles = [{ - type: "test", - schema: { - title: "Fake Profile Type", - description: "Fake Profile Description", - type: "object", - properties: { - test: { + it("should return true for option 'test' when profile schema is set 2", () => { + const mockedProfiles = [{ + type: "test", + schema: { + title: "Fake Profile Type", + description: "Fake Profile Description", + type: "object", + properties: { + test: { + type: "string", + secure: true, + optionDefinitions: [{ + name: "test1", type: "string", - secure: true, - optionDefinitions: [{ - name: "test1", - type: "string", - description: "Fake Test Description" - }, { - name: "test2", - type: "string", - description: "Fake Test Description" - }] - } + description: "Fake Test Description" + }, { + name: "test2", + type: "string", + description: "Fake Test Description" + }] } } - }]; - impConfigSpy.mockReturnValue({ - loadedConfig: { - profiles: mockedProfiles - } - }); - expect(Censor.isSpecialValue("profiles.test.properties.test")).toBe(false); - expect(Censor.isSpecialValue("profiles.test.properties.test1")).toBe(true); - expect(Censor.isSpecialValue("profiles.test.properties.test2")).toBe(true); + } + }]; + impConfigSpy.mockReturnValue({ + loadedConfig: { + profiles: mockedProfiles + } }); + expect(Censor.isSpecialValue("profiles.test.properties.test")).toBe(false); + expect(Censor.isSpecialValue("profiles.test.properties.test1")).toBe(true); + expect(Censor.isSpecialValue("profiles.test.properties.test2")).toBe(true); + }); + }); + + describe("setProfileSchemas", () => { + let impConfigSpy: jest.SpyInstance = null; + + beforeEach(() => { + (Censor as any).mSchema = null; + impConfigSpy = jest.spyOn(ImperativeConfig, "instance", "get"); + }); + + afterAll(() => { + (Censor as any).mSchema = null; + }); + + it("should initialize the schema object with nothing", () => { + expect((Censor as any).mSchema).toBe(null); + Censor.setProfileSchemas([]); + expect((Censor as any).mSchema).toEqual([]); + }); + + it("should set the schema with a map", () => { + const mockedProfiles = { + title: "Fake Profile Type", + description: "Fake Profile Description", + type: "object", + properties: { + test: { + type: "string", + secure: true, + optionDefinitions: [{ + name: "test1", + type: "string", + description: "Fake Test Description" + }, { + name: "test2", + type: "string", + description: "Fake Test Description" + }] + } + } + }; + Censor.setProfileSchemas(new Map([["test", mockedProfiles]])); + expect((Censor as any).mSchema).toEqual([{ type: "object", schema: mockedProfiles }]); + }); + + it("should set the schema with an IProfileTypeConfigration array", () => { + const mockedProfiles = [{ + type: "test", + schema: { + title: "Fake Profile Type", + description: "Fake Profile Description", + type: "object", + properties: { + test: { + type: "string", + secure: true, + optionDefinitions: [{ + name: "test1", + type: "string", + description: "Fake Test Description" + }, { + name: "test2", + type: "string", + description: "Fake Test Description" + }] + } + } + } + }]; + + Censor.setProfileSchemas(mockedProfiles); + expect((Censor as any).mSchema).toEqual([{ type: "test", schema: mockedProfiles[0].schema }]); }); }); }); diff --git a/packages/imperative/src/censor/src/Censor.ts b/packages/imperative/src/censor/src/Censor.ts index ae28f75a53..6af06d987e 100644 --- a/packages/imperative/src/censor/src/Censor.ts +++ b/packages/imperative/src/censor/src/Censor.ts @@ -22,6 +22,11 @@ import { IProfileSchema} from "../../profiles/src/doc/definition/IProfileSchema" import { IProfileTypeConfiguration } from "../../profiles/src/doc/config/IProfileTypeConfiguration"; export class Censor { + + /********************************************************************* + * Basic censorship items - list definitions & initialiazations, etc. * + **********************************************************************/ + /* * NOTE(Kelosky): Ideally we might have a consolidated list for secure fields, but for now we'll just * make sure they're collocated within the same class. @@ -67,48 +72,6 @@ export class Censor { return Array.from(this.censored_options); } - /** - * Specifies whether a given property path (e.g. "profiles.lpar1.properties.host") is a special value or not. - * Special value: Refers to any value defined as secure in the schema definition. - * These values should be already masked by the application (and/or plugin) developer. - * @param prop Property path to determine if it is a special value - * @returns True - if the given property is to be treated as a special value; False - otherwise - */ - public static isSpecialValue(prop: string): boolean { - let specialValues = this.SECURE_PROMPT_OPTIONS; - const getPropertyNames = (prop: ICommandProfileProperty): string[] => { - const ret: string[] = []; - ret.push(prop.optionDefinition?.name); - prop.optionDefinitions?.map(opDef => ret.push(opDef.name)); - return ret; - }; - - for (const profile of this.profileSchemas) { - // eslint-disable-next-line unused-imports/no-unused-vars - for (const [_, prop] of Object.entries(profile.schema.properties)) { - if (prop.secure) specialValues = lodash.union(specialValues, getPropertyNames(prop)); - } - } - - for (const v of specialValues) { - if (prop.endsWith(`.${v}`)) return true; - } - return false; - } - - /** - * Add a censored option, including it's camelCase and kebabCase versions - * @param {string} option - The option to censor - */ - public static addCensoredOption(option: string) { - // This option is required, but we do not want to ever allow null or undefined itself into the censored options - if (option != null) { - this.censored_options.add(option); - this.censored_options.add(CliUtils.getOptionFormat(option).camelCase); - this.censored_options.add(CliUtils.getOptionFormat(option).kebabCase); - } - } - /** * Singleton implementation of an internal reference to the schema */ @@ -124,7 +87,7 @@ export class Censor { /** * Helper method to set an internal reference to loaded profiles - * @param _schemas - The schemas to pass in to set to the logger + * @param _schemas - The schmas to pass in to set to the logger */ public static setProfileSchemas(_schemas: IProfileTypeConfiguration[] | Map) { if (this.mSchema == null) { @@ -141,6 +104,87 @@ export class Censor { } } + /**************************************************** + * Helper functions for more advanced functionality * + ****************************************************/ + + /** + * Helper function to handle profile schemas when setting the censored options + * @param {ICommandProfileTypeConfiguration} profileType - the profile type configuration to iterate over + */ + private static handleSchema(profileType: ICommandProfileTypeConfiguration): void { + /* eslint-disable-next-line no-unused-vars */ + for (const [_, prop] of Object.entries(profileType.schema.properties)) { + // Add censored options from the schema if the option is secure + if (prop.secure) { + // Handle the case of a single option definition + if (prop.optionDefinition) { + this.addCensoredOption(prop.optionDefinition.name); + for (const alias of prop.optionDefinition.aliases || []) { + // Remember to add the alias + this.addCensoredOption(alias); + } + } + + // Handle the case of multiple option definitions + prop.optionDefinitions?.map(opDef => { + this.addCensoredOption(opDef.name); + for (const alias of opDef.aliases || []) { + // Remember to add the alias + this.addCensoredOption(alias); + } + }); + } + } + } + + /** + * Add a censored option, including it's camelCase and kebabCase versions + * @param {string} option - The option to censor + */ + private static addCensoredOption(option: string) { + // This option is required, but we do not want to ever allow null or undefined itself into the censored options + if (option != null) { + this.censored_options.add(option); + this.censored_options.add(CliUtils.getOptionFormat(option).camelCase); + this.censored_options.add(CliUtils.getOptionFormat(option).kebabCase); + } + } + + + /** + * Specifies whether a given property path (e.g. "profiles.lpar1.properties.host") is a special value or not. + * Special value: Refers to any value defined as secure in the schema definition. + * These values should be already masked by the application (and/or plugin) developer. + * @param prop Property path to determine if it is a special value + * @returns True - if the given property is to be treated as a special value; False - otherwise + */ + public static isSpecialValue(prop: string): boolean { + let specialValues = this.SECURE_PROMPT_OPTIONS; + const getPropertyNames = (prop: ICommandProfileProperty): string[] => { + const ret: string[] = []; + ret.push(prop.optionDefinition?.name); + prop.optionDefinitions?.map(opDef => ret.push(opDef.name)); + return ret; + }; + + for (const profile of this.profileSchemas) { + // eslint-disable-next-line unused-imports/no-unused-vars + for (const [_, prop] of Object.entries(profile.schema.properties)) { + if (prop.secure) specialValues = lodash.union(specialValues, getPropertyNames(prop)); + } + } + + for (const v of specialValues) { + if (prop.endsWith(`.${v}`)) return true; + } + return false; + } + + /**************************************************************************************** + * Bread and butter functions, setting up the class and performing censorship of values * + ****************************************************************************************/ + /** * Generate and set the list of censored options. * Attempt to source the censored options from the schema, config, and/or command being executed. @@ -157,28 +201,16 @@ export class Censor { if (censorOpts.profiles) {this.setProfileSchemas(censorOpts.profiles);} for (const profileType of this.profileSchemas ?? []) { - for (const [_, prop] of Object.entries(profileType.schema.properties)) { - // Add censored options from the schema if the option is secure - if (prop.secure) { - // Handle the case of a single option definition - if (prop.optionDefinition) { - this.addCensoredOption(prop.optionDefinition.name); - for (const alias of prop.optionDefinition.aliases || []) { - // Remember to add the alias - this.addCensoredOption(alias); - } - } - - // Handle the case of multiple option definitions - prop.optionDefinitions?.map(opDef => { - this.addCensoredOption(opDef.name); - for (const alias of opDef.aliases || []) { - // Remember to add the alias - this.addCensoredOption(alias); - } - }); - } + // If we know the command we are running, and we know the profile types that the command uses + // we should only use those profiles to determine what should be censored. + if (censorOpts.commandDefinition?.profile?.optional && + !censorOpts.commandDefinition?.profile?.optional?.includes(profileType.type) || + censorOpts.commandDefinition?.profile?.required && + !censorOpts.commandDefinition?.profile?.required?.includes(profileType.type)) { + continue; } + + this.handleSchema(profileType); } // Include any secure options from the config @@ -207,6 +239,10 @@ export class Censor { ); } } + } else if (this.profileSchemas) { + for (const profileType of this.profileSchemas) { + this.handleSchema(profileType); + } } } diff --git a/packages/imperative/src/config/src/ProfileInfo.ts b/packages/imperative/src/config/src/ProfileInfo.ts index e59c64ff25..e8806c8edc 100644 --- a/packages/imperative/src/config/src/ProfileInfo.ts +++ b/packages/imperative/src/config/src/ProfileInfo.ts @@ -1066,7 +1066,9 @@ export class ProfileInfo { } else { schemaJson = lastSchema.json; } - for (const { type, schema } of ConfigSchema.loadSchema(schemaJson)) { + const loadedSchemas = ConfigSchema.loadSchema(schemaJson); + Censor.setProfileSchemas(loadedSchemas); + for (const { type, schema } of loadedSchemas) { this.mProfileSchemaCache.set(`${layer.path}:${type}`, schema); } } catch (error) { @@ -1080,7 +1082,6 @@ export class ProfileInfo { } this.mHasValidSchema = lastSchema.path != null; - Censor.setProfileSchemas(this.mProfileSchemaCache); } /** From d0553d08871a98e64e677894d53077fad3246285 Mon Sep 17 00:00:00 2001 From: "Andrew W. Harn" Date: Mon, 3 Feb 2025 14:09:26 -0500 Subject: [PATCH 07/21] Fix test coverage Signed-off-by: Andrew W. Harn --- .../__tests__/CommandProcessor.unit.test.ts | 40 +++++++++++++++++-- .../src/logger/__tests__/Logger.unit.test.ts | 8 +++- .../logger/__tests__/LoggerUtils.unit.test.ts | 24 +++++++++++ 3 files changed, 67 insertions(+), 5 deletions(-) diff --git a/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts b/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts index 2fea415179..b24cc7bd3b 100644 --- a/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts +++ b/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts @@ -25,7 +25,7 @@ import { setupConfigToLoad } from "../../../__tests__/src/TestUtil"; import { EnvFileUtils } from "../../utilities"; import { join } from "path"; import { Config } from "../../config"; -import { LoggerUtils } from "../../logger/src/LoggerUtils"; +import { Censor } from "../../censor"; jest.mock("../src/syntax/SyntaxValidator"); jest.mock("../src/utils/SharedOptions"); @@ -790,7 +790,7 @@ describe("Command Processor", () => { expect(commandResponse.error?.additionalDetails).toEqual("Syntax validation error!"); }); - it("should mask sensitive CLI options like user and password in log output", async () => { + it("should mask sensitive CLI options like user and password in log output 1", async () => { // Allocate the command processor const processor: CommandProcessor = new CommandProcessor({ envVariablePrefix: ENV_VAR_PREFIX, @@ -824,6 +824,40 @@ describe("Command Processor", () => { expect(logOutput).toContain("--user fakeUser --password **** --token-value **** --cert-file-passphrase **** --cert-key-file /fake/path"); }); + it("should mask sensitive CLI options like user and password in log output 2", async () => { + // Allocate the command processor + const processor: CommandProcessor = new CommandProcessor({ + envVariablePrefix: ENV_VAR_PREFIX, + fullDefinition: SAMPLE_COMPLEX_COMMAND, + definition: SAMPLE_COMMAND_DEFINITION, + helpGenerator: FAKE_HELP_GENERATOR, + rootCommandName: SAMPLE_ROOT_COMMAND, + commandLine: "-u fakeUser --password fakePass --token-value fakeToken " + + "--cert-file-passphrase fakePassphrase --cert-key-file /fake/path", + promptPhrase: "dummydummy" + }); + + // Mock log.info call + let logOutput: string = ""; + const mockLogInfo = jest.fn((line) => { + logOutput += line + "\n"; + }); + Object.defineProperty(processor, "log", { + get: () => ({ + debug: jest.fn(), + error: jest.fn(), + info: mockLogInfo, + trace: jest.fn() + }) + }); + + const parms: any = { arguments: { _: [], $0: "", syntaxThrow: true }, responseFormat: "json", silent: true }; + const commandResponse: ICommandResponse = await processor.invoke(parms); + + expect(mockLogInfo).toHaveBeenCalled(); + expect(logOutput).toContain("-u fakeUser --password **** --token-value **** --cert-file-passphrase **** --cert-key-file /fake/path"); + }); + it("should handle not being able to instantiate the handler", async () => { // Allocate the command processor const processor: CommandProcessor = new CommandProcessor({ @@ -1448,7 +1482,7 @@ describe("Command Processor", () => { expect(commandResponse.data.requiredProfiles).toBeUndefined(); }); - it.each(LoggerUtils.SECURE_PROMPT_OPTIONS)("should mask input value for secure parm %s when --show-inputs-only flag is set", async (propName) => { + it.each(Censor.SECURE_PROMPT_OPTIONS)("should mask input value for secure parm %s when --show-inputs-only flag is set", async (propName) => { // values to test const parm1Key = CliUtils.getOptionFormat(propName).kebabCase; diff --git a/packages/imperative/src/logger/__tests__/Logger.unit.test.ts b/packages/imperative/src/logger/__tests__/Logger.unit.test.ts index 81db1db59d..27ba69f3c1 100644 --- a/packages/imperative/src/logger/__tests__/Logger.unit.test.ts +++ b/packages/imperative/src/logger/__tests__/Logger.unit.test.ts @@ -102,7 +102,7 @@ describe("Logger tests", () => { it("Should allow all service function to store message in memory", () => { const logger = Logger.getImperativeLogger(); - const expectedSize = 6; + const expectedSize = 7; Logger.setLogInMemory(true); logger.trace("test"); @@ -111,6 +111,7 @@ describe("Logger tests", () => { logger.warn("test"); logger.error("test"); logger.fatal("test"); + logger.simple("test"); expect(LoggerManager.instance.QueuedMessages.length).toBe(expectedSize); }); @@ -169,6 +170,7 @@ describe("Logger tests", () => { (logger as any).logService.warn = jest.fn((data: string) => data); (logger as any).logService.error = jest.fn((data: string) => data); (logger as any).logService.fatal = jest.fn((data: string) => data); + (logger as any).logService.simple = jest.fn((data: string) => data); const error = new ImperativeError({msg: "sample error"}); @@ -180,6 +182,7 @@ describe("Logger tests", () => { expect((logger as any).logService.warn).not.toHaveBeenCalledTimes(1); expect((logger as any).logService.fatal).not.toHaveBeenCalled(); expect((logger as any).logService.error).toHaveBeenCalledTimes(2); + expect((logger as any).logService.simple).not.toHaveBeenCalled(); }); it("Should get the correct requested logger appender", () => { @@ -270,7 +273,7 @@ describe("Logger tests", () => { it("Should support writing all of the message in memory to file", () => { const logger = Logger.getImperativeLogger(); - const expectedSize = 6; + const expectedSize = 7; Logger.setLogInMemory(true); logger.trace("test"); @@ -279,6 +282,7 @@ describe("Logger tests", () => { logger.warn("test"); logger.error("test"); logger.fatal("test"); + logger.simple("test"); expect(LoggerManager.instance.QueuedMessages.length).toBe(expectedSize); diff --git a/packages/imperative/src/logger/__tests__/LoggerUtils.unit.test.ts b/packages/imperative/src/logger/__tests__/LoggerUtils.unit.test.ts index 653cd50771..70c15247f9 100644 --- a/packages/imperative/src/logger/__tests__/LoggerUtils.unit.test.ts +++ b/packages/imperative/src/logger/__tests__/LoggerUtils.unit.test.ts @@ -15,6 +15,7 @@ jest.mock("../../config/src/Config"); import { EnvironmentalVariableSettings } from "../../imperative/src/env/EnvironmentalVariableSettings"; import { LoggerUtils } from "../src/LoggerUtils"; import { ImperativeConfig } from "../../utilities/src/ImperativeConfig"; +import { Censor } from "../.."; afterAll(() => { jest.restoreAllMocks(); @@ -145,4 +146,27 @@ describe("LoggerUtils tests", () => { }); }); }); + + describe("isSpecialValue", () => { + let impConfigSpy: jest.SpyInstance = null; + + beforeEach(() => { + (Censor as any).mSchema = null; + impConfigSpy = jest.spyOn(ImperativeConfig, "instance", "get"); + }); + + afterAll(() => { + (Censor as any).mSchema = null; + }); + + it("should check if user is a special value", () => { + expect(LoggerUtils.isSpecialValue("profiles.test.properties.user")).toBe(true); + }); + }); + + it("should get profile schemas from Censor", () => { + const schemaSpy = jest.spyOn(Censor, "profileSchemas", "get"); + expect(LoggerUtils.profileSchemas).toEqual(Censor.profileSchemas); + expect(schemaSpy).toHaveBeenCalledTimes(2); + }); }); From 64af500a7bf9b7befb8f9c27298e0321ac5e2f03 Mon Sep 17 00:00:00 2001 From: "Andrew W. Harn" Date: Mon, 3 Feb 2025 14:39:28 -0500 Subject: [PATCH 08/21] Try to increase coverage Signed-off-by: Andrew W. Harn --- .../src/cmd/__tests__/CommandProcessor.unit.test.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts b/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts index b24cc7bd3b..6c8daabc27 100644 --- a/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts +++ b/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts @@ -818,13 +818,16 @@ describe("Command Processor", () => { }); const parms: any = { arguments: { _: [], $0: "", syntaxThrow: true }, responseFormat: "json", silent: true }; - const commandResponse: ICommandResponse = await processor.invoke(parms); + await processor.invoke(parms); expect(mockLogInfo).toHaveBeenCalled(); expect(logOutput).toContain("--user fakeUser --password **** --token-value **** --cert-file-passphrase **** --cert-key-file /fake/path"); }); it("should mask sensitive CLI options like user and password in log output 2", async () => { + const realCensoredOptions = Censor.CENSORED_OPTIONS; + jest.spyOn(Censor, "CENSORED_OPTIONS", "get").mockReturnValueOnce([...realCensoredOptions, "u"]); + // Allocate the command processor const processor: CommandProcessor = new CommandProcessor({ envVariablePrefix: ENV_VAR_PREFIX, @@ -852,10 +855,10 @@ describe("Command Processor", () => { }); const parms: any = { arguments: { _: [], $0: "", syntaxThrow: true }, responseFormat: "json", silent: true }; - const commandResponse: ICommandResponse = await processor.invoke(parms); + await processor.invoke(parms); expect(mockLogInfo).toHaveBeenCalled(); - expect(logOutput).toContain("-u fakeUser --password **** --token-value **** --cert-file-passphrase **** --cert-key-file /fake/path"); + expect(logOutput).toContain("-u **** --password **** --token-value **** --cert-file-passphrase **** --cert-key-file /fake/path"); }); it("should handle not being able to instantiate the handler", async () => { From d15e82344ab270816cd23443e450d58e37519554 Mon Sep 17 00:00:00 2001 From: "Andrew W. Harn" Date: Mon, 3 Feb 2025 15:04:34 -0500 Subject: [PATCH 09/21] handleSchema tests Signed-off-by: Andrew W. Harn --- .../src/censor/__tests__/Censor.unit.test.ts | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/packages/imperative/src/censor/__tests__/Censor.unit.test.ts b/packages/imperative/src/censor/__tests__/Censor.unit.test.ts index e80d6c7a21..552e280439 100644 --- a/packages/imperative/src/censor/__tests__/Censor.unit.test.ts +++ b/packages/imperative/src/censor/__tests__/Censor.unit.test.ts @@ -450,4 +450,64 @@ describe("Censor tests", () => { expect((Censor as any).mSchema).toEqual([{ type: "test", schema: mockedProfiles[0].schema }]); }); }); + + describe("handleSchema", () => { + it("should add secure props with an option definition to the secure array", () => { + const fakeSchema = { + type: "test", + schema: { + title: "Fake Profile Type", + description: "Fake Profile Description", + type: "object", + properties: { + test: { + type: "string", + secure: true, + optionDefinition: { + name: "test", + type: "string", + description: "Fake Test Description", + aliases: ["test1", "t"] + } + } + } + } + }; + (Censor as any).handleSchema(fakeSchema); + expect(Censor.CENSORED_OPTIONS).toContain("test"); + expect(Censor.CENSORED_OPTIONS).toContain("test1"); + expect(Censor.CENSORED_OPTIONS).toContain("t"); + }); + + it("should add secure props with option definitions to the secure array", () => { + const fakeSchema = { + type: "test", + schema: { + title: "Fake Profile Type", + description: "Fake Profile Description", + type: "object", + properties: { + test: { + type: "string", + secure: true, + optionDefinitions: [{ + name: "test1", + type: "string", + description: "Fake Test Description" + }, { + name: "test2", + type: "string", + description: "Fake Test Description", + aliases: ["t"] + }] + } + } + } + }; + (Censor as any).handleSchema(fakeSchema); + expect(Censor.CENSORED_OPTIONS).toContain("test1"); + expect(Censor.CENSORED_OPTIONS).toContain("test2"); + expect(Censor.CENSORED_OPTIONS).toContain("t"); + }); + }); }); From dcad7cb8a24d133fd2e1ea378135f96d2a8d35ce Mon Sep 17 00:00:00 2001 From: "Andrew W. Harn" Date: Mon, 3 Feb 2025 16:48:44 -0500 Subject: [PATCH 10/21] Add additional tests Signed-off-by: Andrew W. Harn --- .../src/censor/__tests__/Censor.unit.test.ts | 237 ++++++++++++++++++ packages/imperative/src/censor/src/Censor.ts | 43 ++-- .../src/censor/src/doc/ICensorOptions.ts | 4 +- 3 files changed, 261 insertions(+), 23 deletions(-) diff --git a/packages/imperative/src/censor/__tests__/Censor.unit.test.ts b/packages/imperative/src/censor/__tests__/Censor.unit.test.ts index 552e280439..534eb3038b 100644 --- a/packages/imperative/src/censor/__tests__/Censor.unit.test.ts +++ b/packages/imperative/src/censor/__tests__/Censor.unit.test.ts @@ -15,13 +15,16 @@ jest.mock("../../config/src/Config"); import { EnvironmentalVariableSettings } from "../../imperative/src/env/EnvironmentalVariableSettings"; import { Censor } from "../src/Censor"; import { ImperativeConfig } from "../../utilities/src/ImperativeConfig"; +import { ICensorOptions } from "../.."; beforeAll(() => { + (Censor as any).mSchema = null; Censor.setCensoredOptions(); }); afterAll(() => { jest.restoreAllMocks(); + (Censor as any).mSchema = null; Censor.setCensoredOptions(); }); @@ -107,6 +110,11 @@ describe("Censor tests", () => { Censor.setProfileSchemas(new Map()); }); + afterAll(() => { + impConfigSpy.mockRestore(); + envSettingsReadSpy.mockRestore(); + }); + describe("should NOT censor", () => { it("data if we are using old profiles 1", () => { impConfigSpy.mockReturnValue({ config: { exists: false }, envVariablePrefix: "ZOWE" }); @@ -247,6 +255,7 @@ describe("Censor tests", () => { afterAll(() => { (Censor as any).mSchema = null; + impConfigSpy.mockRestore(); }); it("should return the schema from ImperativeConfig", () => { @@ -292,6 +301,7 @@ describe("Censor tests", () => { afterAll(() => { (Censor as any).mSchema = null; + impConfigSpy.mockRestore(); }); describe("default list", () => { @@ -388,6 +398,7 @@ describe("Censor tests", () => { afterAll(() => { (Censor as any).mSchema = null; + impConfigSpy.mockRestore(); }); it("should initialize the schema object with nothing", () => { @@ -452,6 +463,12 @@ describe("Censor tests", () => { }); describe("handleSchema", () => { + + beforeEach(() => { + Censor.setCensoredOptions(); + (Censor as any).mSchema = null; + }); + it("should add secure props with an option definition to the secure array", () => { const fakeSchema = { type: "test", @@ -510,4 +527,224 @@ describe("Censor tests", () => { expect(Censor.CENSORED_OPTIONS).toContain("t"); }); }); + + describe("setCensoredOptions", () => { + + beforeEach(() => { + (Censor as any).mSchema = null; + Censor.setCensoredOptions(); + }); + + it("should reset the censored options when called with nothing", () => { + (Censor as any).mConfig = "bad"; + (Censor as any).setCensoredOptions({}); + expect((Censor as any).mConfig).not.toEqual("bad"); + }); + + it("should apply known profile schemas", () => { + (Censor as any).mSchema = [{ + type: "test", + schema: { + title: "Fake Profile Type", + description: "Fake Profile Description", + type: "object", + properties: { + test: { + type: "string", + secure: true, + optionDefinitions: [{ + name: "test1", + type: "string", + description: "Fake Test Description" + }, { + name: "test2", + type: "string", + description: "Fake Test Description", + aliases: ["t"] + }] + } + } + } + }]; + expect(Censor.CENSORED_OPTIONS).not.toContain("test1"); + expect(Censor.CENSORED_OPTIONS).not.toContain("test2"); + expect(Censor.CENSORED_OPTIONS).not.toContain("t"); + + Censor.setCensoredOptions(); + + expect(Censor.CENSORED_OPTIONS).toContain("test1"); + expect(Censor.CENSORED_OPTIONS).toContain("test2"); + expect(Censor.CENSORED_OPTIONS).toContain("t"); + }); + + it("should apply a profile schema if the command definition uses it (required)", () => { + const censorOpts: ICensorOptions = { + profiles: [{ + type: "test", + schema: { + title: "Fake Profile Type", + description: "Fake Profile Description", + type: "object", + properties: { + test: { + type: "string", + secure: true, + optionDefinitions: [{ + name: "test1", + type: "string", + description: "Fake Test Description" + }, { + name: "test2", + type: "string", + description: "Fake Test Description", + aliases: ["t"] + }] + } + } + } + }], + commandDefinition: { + name: "test", + description: "test", + type: "command", + profile: { + required: ["test"] + } + }, + } + + Censor.setCensoredOptions(censorOpts); + + expect(Censor.CENSORED_OPTIONS).toContain("test1"); + expect(Censor.CENSORED_OPTIONS).toContain("test2"); + expect(Censor.CENSORED_OPTIONS).toContain("t"); + }); + + it("should apply a profile schema if the command definition uses it (optional)", () => { + const censorOpts: ICensorOptions = { + profiles: [{ + type: "test", + schema: { + title: "Fake Profile Type", + description: "Fake Profile Description", + type: "object", + properties: { + test: { + type: "string", + secure: true, + optionDefinitions: [{ + name: "test1", + type: "string", + description: "Fake Test Description" + }, { + name: "test2", + type: "string", + description: "Fake Test Description", + aliases: ["t"] + }] + } + } + } + }], + commandDefinition: { + name: "test", + description: "test", + type: "command", + profile: { + optional: ["test"] + } + }, + } + + Censor.setCensoredOptions(censorOpts); + + expect(Censor.CENSORED_OPTIONS).toContain("test1"); + expect(Censor.CENSORED_OPTIONS).toContain("test2"); + expect(Censor.CENSORED_OPTIONS).toContain("t"); + }); + + it("should not apply a profile schema if the command definition doesn't use any profiles", () => { + const censorOpts: ICensorOptions = { + profiles: [{ + type: "test", + schema: { + title: "Fake Profile Type", + description: "Fake Profile Description", + type: "object", + properties: { + test: { + type: "string", + secure: true, + optionDefinitions: [{ + name: "test1", + type: "string", + description: "Fake Test Description" + }, { + name: "test2", + type: "string", + description: "Fake Test Description", + aliases: ["t"] + }] + } + } + } + }], + commandDefinition: { + name: "test", + description: "test", + type: "command", + profile: {} + }, + } + + Censor.setCensoredOptions(censorOpts); + + expect(Censor.CENSORED_OPTIONS).not.toContain("test1"); + expect(Censor.CENSORED_OPTIONS).not.toContain("test2"); + expect(Censor.CENSORED_OPTIONS).not.toContain("t"); + }); + + it("should not apply a profile schema if the command definition does not use it", () => { + const censorOpts: ICensorOptions = { + profiles: [{ + type: "test", + schema: { + title: "Fake Profile Type", + description: "Fake Profile Description", + type: "object", + properties: { + test: { + type: "string", + secure: true, + optionDefinitions: [{ + name: "test1", + type: "string", + description: "Fake Test Description" + }, { + name: "test2", + type: "string", + description: "Fake Test Description", + aliases: ["t"] + }] + } + } + } + }], + commandDefinition: { + name: "test", + description: "test", + type: "command", + profile: { + optional: ["nottest"] + } + }, + } + + Censor.setCensoredOptions(censorOpts); + + expect(Censor.CENSORED_OPTIONS).not.toContain("test1"); + expect(Censor.CENSORED_OPTIONS).not.toContain("test2"); + expect(Censor.CENSORED_OPTIONS).not.toContain("t"); + }); + }); }); diff --git a/packages/imperative/src/censor/src/Censor.ts b/packages/imperative/src/censor/src/Censor.ts index 6af06d987e..0ee0552654 100644 --- a/packages/imperative/src/censor/src/Censor.ts +++ b/packages/imperative/src/censor/src/Censor.ts @@ -110,11 +110,12 @@ export class Censor { /** * Helper function to handle profile schemas when setting the censored options - * @param {ICommandProfileTypeConfiguration} profileType - the profile type configuration to iterate over + * @param {IProfileTypeConfiguration | ICommandProfileTypeConfiguration} profileType - the profile type configuration to iterate over */ - private static handleSchema(profileType: ICommandProfileTypeConfiguration): void { + private static handleSchema(profileType: IProfileTypeConfiguration | ICommandProfileTypeConfiguration): void { /* eslint-disable-next-line no-unused-vars */ - for (const [_, prop] of Object.entries(profileType.schema.properties)) { + for (const [key, cmdProp] of Object.entries(profileType.schema.properties)) { + const prop = cmdProp as ICommandProfileProperty; // Add censored options from the schema if the option is secure if (prop.secure) { // Handle the case of a single option definition @@ -124,16 +125,18 @@ export class Censor { // Remember to add the alias this.addCensoredOption(alias); } + } else if (prop.optionDefinitions) { + // Handle the case of multiple option definitions + prop.optionDefinitions.map(opDef => { + this.addCensoredOption(opDef.name); + for (const alias of opDef.aliases || []) { + // Remember to add the alias + this.addCensoredOption(alias); + } + }); + } else { + this.addCensoredOption(key); } - - // Handle the case of multiple option definitions - prop.optionDefinitions?.map(opDef => { - this.addCensoredOption(opDef.name); - for (const alias of opDef.aliases || []) { - // Remember to add the alias - this.addCensoredOption(alias); - } - }); } } } @@ -198,19 +201,17 @@ export class Censor { this.mConfig = censorOpts.config; // If we have a ProfileTypeConfiguration (i.e. ImperativeConfig.instance.loadedConfig.profiles) - if (censorOpts.profiles) {this.setProfileSchemas(censorOpts.profiles);} + if (censorOpts.profiles) { this.setProfileSchemas(censorOpts.profiles); } for (const profileType of this.profileSchemas ?? []) { // If we know the command we are running, and we know the profile types that the command uses - // we should only use those profiles to determine what should be censored. - if (censorOpts.commandDefinition?.profile?.optional && - !censorOpts.commandDefinition?.profile?.optional?.includes(profileType.type) || - censorOpts.commandDefinition?.profile?.required && - !censorOpts.commandDefinition?.profile?.required?.includes(profileType.type)) { - continue; + // we should only use those profiles to determine what should be censored. If we do not, we should + // add everything + if (censorOpts.commandDefinition == null || + censorOpts.commandDefinition.profile?.optional?.includes(profileType.type) || + censorOpts.commandDefinition.profile?.required?.includes(profileType.type)) { + this.handleSchema(profileType); } - - this.handleSchema(profileType); } // Include any secure options from the config diff --git a/packages/imperative/src/censor/src/doc/ICensorOptions.ts b/packages/imperative/src/censor/src/doc/ICensorOptions.ts index d2e63baac8..7afd803661 100644 --- a/packages/imperative/src/censor/src/doc/ICensorOptions.ts +++ b/packages/imperative/src/censor/src/doc/ICensorOptions.ts @@ -9,7 +9,7 @@ * */ -import { ICommandDefinition, ICommandArguments } from "../../../cmd"; +import { ICommandDefinition, ICommandArguments, ICommandProfileTypeConfiguration } from "../../../cmd"; import { Config } from "../../../config"; import { IProfileTypeConfiguration } from "../../../profiles"; @@ -17,7 +17,7 @@ export interface ICensorOptions { /** * An array of profile schema definitions */ - profiles?: IProfileTypeConfiguration[]; + profiles?: IProfileTypeConfiguration[] | ICommandProfileTypeConfiguration[]; /** * The team config API From 01d643ad6e3dd8d5099575084a0dbccfd4538d19 Mon Sep 17 00:00:00 2001 From: "Andrew W. Harn" Date: Wed, 5 Feb 2025 16:54:47 -0500 Subject: [PATCH 11/21] Refactor and update tests Signed-off-by: Andrew W. Harn --- .../src/censor/__tests__/Censor.unit.test.ts | 192 +++++++++++++++++- packages/imperative/src/censor/src/Censor.ts | 81 +++++--- 2 files changed, 246 insertions(+), 27 deletions(-) diff --git a/packages/imperative/src/censor/__tests__/Censor.unit.test.ts b/packages/imperative/src/censor/__tests__/Censor.unit.test.ts index 534eb3038b..f45c655bff 100644 --- a/packages/imperative/src/censor/__tests__/Censor.unit.test.ts +++ b/packages/imperative/src/censor/__tests__/Censor.unit.test.ts @@ -16,6 +16,7 @@ import { EnvironmentalVariableSettings } from "../../imperative/src/env/Environm import { Censor } from "../src/Censor"; import { ImperativeConfig } from "../../utilities/src/ImperativeConfig"; import { ICensorOptions } from "../.."; +import { ConfigSecure } from "../../config/src/api"; beforeAll(() => { (Censor as any).mSchema = null; @@ -537,7 +538,7 @@ describe("Censor tests", () => { it("should reset the censored options when called with nothing", () => { (Censor as any).mConfig = "bad"; - (Censor as any).setCensoredOptions({}); + Censor.setCensoredOptions({}); expect((Censor as any).mConfig).not.toEqual("bad"); }); @@ -746,5 +747,194 @@ describe("Censor tests", () => { expect(Censor.CENSORED_OPTIONS).not.toContain("test2"); expect(Censor.CENSORED_OPTIONS).not.toContain("t"); }); + + it("should apply a config without command definitions", () => { + const censorOpts: ICensorOptions = { + config: { + api: { + secure: new ConfigSecure({} as any) + }, + mProperties: { + profiles: { + test1: { + secure: [ + "host", + "port", + "user" + ] + } + } + } + } as any + } + Censor.setCensoredOptions(censorOpts); + + expect(Censor.CENSORED_OPTIONS).toContain("host"); + expect(Censor.CENSORED_OPTIONS).toContain("port"); + expect(Censor.CENSORED_OPTIONS).toContain("user"); + expect(Censor.CENSORED_OPTIONS).toContain("password"); + }); + + it("should apply a config with command definitions 1", () => { + const profile = { + test1: { + secure: [ + "host", + "port", + "user" + ] + } + } + const censorOpts: ICensorOptions = { + config: { + api: { + secure: new ConfigSecure({ + api: { + profiles: { + getProfilePathFromName: jest.fn().mockReturnValue("profiles.test1") + } + }, + mProperties: { + profiles: { + ...profile + } + } + } as any), + profiles: { + get: jest.fn().mockReturnValue(profile) + } + }, + mProperties: { + profiles: { + ...profile + } + } + } as any, + commandDefinition: { + profile: { + required: [ + "test" + ] + } + } as any, + commandArguments: { + "test-profile": "test1" + } as any + } + Censor.setCensoredOptions(censorOpts); + + expect(Censor.CENSORED_OPTIONS).toContain("host"); + expect(Censor.CENSORED_OPTIONS).toContain("port"); + expect(Censor.CENSORED_OPTIONS).toContain("user"); + expect(Censor.CENSORED_OPTIONS).toContain("password"); + }); + + it("should apply a config with command definitions 2", () => { + const profile = { + test1: { + secure: [ + "host", + "port", + "user" + ] + } + } + const censorOpts: ICensorOptions = { + config: { + api: { + secure: new ConfigSecure({ + api: { + profiles: { + getProfilePathFromName: jest.fn().mockReturnValue("profiles.test1") + } + }, + mProperties: { + profiles: { + ...profile + } + } + } as any), + profiles: { + get: jest.fn().mockReturnValue(profile) + } + }, + mProperties: { + profiles: { + ...profile + } + } + } as any, + commandDefinition: { + profile: { + optional: [ + "test" + ] + } + } as any, + commandArguments: { + "test-profile": "test1" + } as any + } + Censor.setCensoredOptions(censorOpts); + + expect(Censor.CENSORED_OPTIONS).toContain("host"); + expect(Censor.CENSORED_OPTIONS).toContain("port"); + expect(Censor.CENSORED_OPTIONS).toContain("user"); + expect(Censor.CENSORED_OPTIONS).toContain("password"); + }); + + it("should not apply a config with command definitions if the profile does not apply", () => { + const profile = { + test1: { + secure: [ + "host", + "port", + "user" + ] + } + } + const censorOpts: ICensorOptions = { + config: { + api: { + secure: new ConfigSecure({ + api: { + profiles: { + getProfilePathFromName: jest.fn().mockReturnValue("profiles.test1") + } + }, + mProperties: { + profiles: { + ...profile + } + } + } as any), + profiles: { + get: jest.fn().mockReturnValue(profile) + } + }, + mProperties: { + profiles: { + ...profile + } + } + } as any, + commandDefinition: { + profile: { + required: [ + "nottest" + ] + } + } as any, + commandArguments: { + "test-profile": "test1" + } as any + } + Censor.setCensoredOptions(censorOpts); + + expect(Censor.CENSORED_OPTIONS).not.toContain("host"); + expect(Censor.CENSORED_OPTIONS).not.toContain("port"); + expect(Censor.CENSORED_OPTIONS).not.toContain("user"); + expect(Censor.CENSORED_OPTIONS).toContain("password"); + }); }); }); diff --git a/packages/imperative/src/censor/src/Censor.ts b/packages/imperative/src/censor/src/Censor.ts index 0ee0552654..56b03adefb 100644 --- a/packages/imperative/src/censor/src/Censor.ts +++ b/packages/imperative/src/censor/src/Censor.ts @@ -20,6 +20,7 @@ import { EnvironmentalVariableSettings } from "../../imperative/src/env/Environm import { ICommandProfileTypeConfiguration } from "../../cmd/src/doc/profiles/definition/ICommandProfileTypeConfiguration"; import { IProfileSchema} from "../../profiles/src/doc/definition/IProfileSchema"; import { IProfileTypeConfiguration } from "../../profiles/src/doc/config/IProfileTypeConfiguration"; +import { ICommandArguments, ICommandDefinition } from "../../cmd"; export class Censor { @@ -64,14 +65,16 @@ export class Censor { // Set a censored options list that can be set and retrieved for each command. private static censored_options: Set = new Set(this.DEFAULT_CENSORED_OPTIONS); - // Keep a cached config object if provided in another function - private static mConfig: Config = null; - // Return a customized list of censored options (or just the defaults if not set). public static get CENSORED_OPTIONS(): string[] { return Array.from(this.censored_options); } + //Singleton caches of the Config, Command Definition and Command Arguments + private static mConfig: Config = null; + private static mCommandDefinition: ICommandDefinition = null; + private static mCommandArguments: ICommandArguments = null; + /** * Singleton implementation of an internal reference to the schema */ @@ -184,6 +187,15 @@ export class Censor { return false; } + /** + * Identifies if a property is a secure property + * @param {string} prop - The property to check + * @returns {boolean} - True if the property is secure; False otherwise + */ + public static isSecureValue(prop: string) { + return this.SECURE_PROMPT_OPTIONS.includes(prop); + } + /**************************************************************************************** * Bread and butter functions, setting up the class and performing censorship of values * ****************************************************************************************/ @@ -216,29 +228,10 @@ export class Censor { // Include any secure options from the config if (censorOpts.config) { - // Try to use the command and inputs to find the profiles being loaded - if (censorOpts.commandDefinition && censorOpts.commandArguments) { - const profiles = []; - for (const prof of censorOpts.commandDefinition.profile?.required || []) { - profiles.push(prof); - } - for (const prof of censorOpts.commandDefinition.profile?.optional || []) { - profiles.push(prof); - } + this.mCommandArguments = censorOpts.commandArguments; + this.mCommandDefinition = censorOpts.commandDefinition; - for (const prof of profiles) { - // If the profile exists, append all of the secure props to the censored list - const profName = censorOpts.commandArguments?.[`${prof}-profile`]; - if (profName && censorOpts.config.api.profiles.get(profName)) { - censorOpts.config.api.secure.securePropsForProfile(profName).forEach(prop => this.addCensoredOption(prop)); - } - } - } else { - // We only have a configuration file, assume every property that is secured should be censored - censorOpts.config.api.secure.findSecure(censorOpts.config.mProperties.profiles, "profiles").forEach( - prop => this.addCensoredOption(prop.split(".").pop()) - ); - } + this.censorFromCommandOrConfig(); } } else if (this.profileSchemas) { for (const profileType of this.profileSchemas) { @@ -247,6 +240,40 @@ export class Censor { } } + /** + * Censor from the command cache from setCensoredOptions, or use the user's config to censor anything secure + */ + private static censorFromCommandOrConfig() { + const config = this.mConfig ?? ImperativeConfig.instance?.config; + + if (config) { + // Try to use the command and inputs to find the profiles being loaded + if (this.mCommandDefinition && this.mCommandArguments) { + const profiles = []; + for (const prof of this.mCommandDefinition.profile?.required || []) { + profiles.push(prof); + } + for (const prof of this.mCommandDefinition.profile?.optional || []) { + profiles.push(prof); + } + + for (const prof of profiles) { + // If the profile exists, append all of the secure props to the censored list + let profName = this.mCommandArguments?.[`${prof}-profile`]; + if (!profName) { profName = this.mConfig.mProperties.defaults[`${prof}`]; } + if (profName && config.api.profiles.get(profName)) { + config.api.secure.securePropsForProfile(profName).forEach(prop => this.addCensoredOption(prop)); + } + } + } else { + // We only have a configuration file, assume every property that is secured should be censored + config.api.secure.findSecure(config.mProperties.profiles, "profiles").forEach( + prop => this.addCensoredOption(prop.split(".").pop()) + ); + } + } + } + /** * Copy and censor any sensitive CLI arguments before logging/printing * @param {string[]} args - The args list to censor @@ -286,10 +313,12 @@ export class Censor { let newData = data; + this.censorFromCommandOrConfig(); + const secureFields = config.api.secure.findSecure(config.mProperties.profiles, "profiles"); for (const prop of secureFields) { const sec = lodash.get(config.mProperties, prop); - if (sec && typeof sec !== "object" && !this.isSpecialValue(prop)) { + if (sec && typeof sec !== "object" && !this.isSpecialValue(prop) && this.isSecureValue(prop.split(".").pop())) { newData = newData.replace(new RegExp(sec, "gi"), this.CENSOR_RESPONSE); } } From 2a4aab7ae60fc7cc00132880b3d361c7383e68ef Mon Sep 17 00:00:00 2001 From: "Andrew W. Harn" Date: Thu, 6 Feb 2025 10:22:33 -0500 Subject: [PATCH 12/21] Refactor code to optimize for config property duplication Signed-off-by: Andrew W. Harn --- packages/imperative/src/censor/src/Censor.ts | 83 +++++++++----------- 1 file changed, 35 insertions(+), 48 deletions(-) diff --git a/packages/imperative/src/censor/src/Censor.ts b/packages/imperative/src/censor/src/Censor.ts index 56b03adefb..09754540fc 100644 --- a/packages/imperative/src/censor/src/Censor.ts +++ b/packages/imperative/src/censor/src/Censor.ts @@ -20,7 +20,6 @@ import { EnvironmentalVariableSettings } from "../../imperative/src/env/Environm import { ICommandProfileTypeConfiguration } from "../../cmd/src/doc/profiles/definition/ICommandProfileTypeConfiguration"; import { IProfileSchema} from "../../profiles/src/doc/definition/IProfileSchema"; import { IProfileTypeConfiguration } from "../../profiles/src/doc/config/IProfileTypeConfiguration"; -import { ICommandArguments, ICommandDefinition } from "../../cmd"; export class Censor { @@ -72,8 +71,6 @@ export class Censor { //Singleton caches of the Config, Command Definition and Command Arguments private static mConfig: Config = null; - private static mCommandDefinition: ICommandDefinition = null; - private static mCommandArguments: ICommandArguments = null; /** * Singleton implementation of an internal reference to the schema @@ -116,6 +113,8 @@ export class Censor { * @param {IProfileTypeConfiguration | ICommandProfileTypeConfiguration} profileType - the profile type configuration to iterate over */ private static handleSchema(profileType: IProfileTypeConfiguration | ICommandProfileTypeConfiguration): void { + const secureOptions: Set = new Set(); + /* eslint-disable-next-line no-unused-vars */ for (const [key, cmdProp] of Object.entries(profileType.schema.properties)) { const prop = cmdProp as ICommandProfileProperty; @@ -123,25 +122,27 @@ export class Censor { if (prop.secure) { // Handle the case of a single option definition if (prop.optionDefinition) { - this.addCensoredOption(prop.optionDefinition.name); + secureOptions.add(prop.optionDefinition.name); for (const alias of prop.optionDefinition.aliases || []) { // Remember to add the alias - this.addCensoredOption(alias); + secureOptions.add(alias); } } else if (prop.optionDefinitions) { // Handle the case of multiple option definitions prop.optionDefinitions.map(opDef => { - this.addCensoredOption(opDef.name); + secureOptions.add(opDef.name); for (const alias of opDef.aliases || []) { // Remember to add the alias - this.addCensoredOption(alias); + secureOptions.add(alias); } }); } else { - this.addCensoredOption(key); + secureOptions.add(key); } } } + + secureOptions.forEach(prop => this.addCensoredOption(prop)); } /** @@ -157,7 +158,6 @@ export class Censor { } } - /** * Specifies whether a given property path (e.g. "profiles.lpar1.properties.host") is a special value or not. * Special value: Refers to any value defined as secure in the schema definition. @@ -228,10 +228,33 @@ export class Censor { // Include any secure options from the config if (censorOpts.config) { - this.mCommandArguments = censorOpts.commandArguments; - this.mCommandDefinition = censorOpts.commandDefinition; + const secureOptions: Set = new Set(); + + // Try to use the command and inputs to find the profiles being loaded + if (censorOpts.commandDefinition && censorOpts.commandArguments) { + const profiles = []; + for (const prof of censorOpts.commandDefinition.profile?.required || []) { + profiles.push(prof); + } + for (const prof of censorOpts.commandDefinition.profile?.optional || []) { + profiles.push(prof); + } - this.censorFromCommandOrConfig(); + for (const prof of profiles) { + // If the profile exists, append all of the secure props to the censored list + let profName = censorOpts.commandArguments?.[`${prof}-profile`]; + if (!profName) { profName = this.mConfig.mProperties.defaults[`${prof}`]; } + if (profName && censorOpts.config.api.profiles.get(profName)) { + censorOpts.config.api.secure.securePropsForProfile(profName).forEach(prop => secureOptions.add(prop)); + } + } + } else { + // We only have a configuration file, assume every property that is secured should be censored + censorOpts.config.api.secure.findSecure(censorOpts.config.mProperties.profiles, "profiles").forEach( + prop => secureOptions.add(prop.split(".").pop()) + ); + } + secureOptions.forEach(prop => this.addCensoredOption(prop)); } } else if (this.profileSchemas) { for (const profileType of this.profileSchemas) { @@ -240,40 +263,6 @@ export class Censor { } } - /** - * Censor from the command cache from setCensoredOptions, or use the user's config to censor anything secure - */ - private static censorFromCommandOrConfig() { - const config = this.mConfig ?? ImperativeConfig.instance?.config; - - if (config) { - // Try to use the command and inputs to find the profiles being loaded - if (this.mCommandDefinition && this.mCommandArguments) { - const profiles = []; - for (const prof of this.mCommandDefinition.profile?.required || []) { - profiles.push(prof); - } - for (const prof of this.mCommandDefinition.profile?.optional || []) { - profiles.push(prof); - } - - for (const prof of profiles) { - // If the profile exists, append all of the secure props to the censored list - let profName = this.mCommandArguments?.[`${prof}-profile`]; - if (!profName) { profName = this.mConfig.mProperties.defaults[`${prof}`]; } - if (profName && config.api.profiles.get(profName)) { - config.api.secure.securePropsForProfile(profName).forEach(prop => this.addCensoredOption(prop)); - } - } - } else { - // We only have a configuration file, assume every property that is secured should be censored - config.api.secure.findSecure(config.mProperties.profiles, "profiles").forEach( - prop => this.addCensoredOption(prop.split(".").pop()) - ); - } - } - } - /** * Copy and censor any sensitive CLI arguments before logging/printing * @param {string[]} args - The args list to censor @@ -313,8 +302,6 @@ export class Censor { let newData = data; - this.censorFromCommandOrConfig(); - const secureFields = config.api.secure.findSecure(config.mProperties.profiles, "profiles"); for (const prop of secureFields) { const sec = lodash.get(config.mProperties, prop); From b5dd0f027f400da76da722a39f5655d651be2450 Mon Sep 17 00:00:00 2001 From: "Andrew W. Harn" Date: Thu, 6 Feb 2025 11:52:55 -0500 Subject: [PATCH 13/21] Fix tests Signed-off-by: Andrew W. Harn --- .../imperative/src/censor/__tests__/Censor.unit.test.ts | 9 ++++++--- packages/imperative/src/censor/src/Censor.ts | 2 +- .../src/cmd/__tests__/CommandProcessor.unit.test.ts | 3 ++- .../src/logger/__tests__/LoggerUtils.unit.test.ts | 3 ++- 4 files changed, 11 insertions(+), 6 deletions(-) diff --git a/packages/imperative/src/censor/__tests__/Censor.unit.test.ts b/packages/imperative/src/censor/__tests__/Censor.unit.test.ts index f45c655bff..ec083fd8e8 100644 --- a/packages/imperative/src/censor/__tests__/Censor.unit.test.ts +++ b/packages/imperative/src/censor/__tests__/Censor.unit.test.ts @@ -149,9 +149,10 @@ describe("Censor tests", () => { describe("special value:", () => { beforeEach(() => { - impConfig.config.mProperties = {profiles: {secret: { properties: {}}}}; + impConfig.config.mProperties = {profiles: {secret: { properties: {}}}, defaults: {}}; impConfigSpy.mockReturnValue(impConfig); envSettingsReadSpy.mockReturnValue({ maskOutput: { value: "TRUE" } }); + (Censor as any).addCensoredOption("secret"); }); const _lazyTest = (prop: string): [string, string] => { @@ -905,7 +906,8 @@ describe("Censor tests", () => { mProperties: { profiles: { ...profile - } + }, + defaults: {} } } as any), profiles: { @@ -915,7 +917,8 @@ describe("Censor tests", () => { mProperties: { profiles: { ...profile - } + }, + defaults: {} } } as any, commandDefinition: { diff --git a/packages/imperative/src/censor/src/Censor.ts b/packages/imperative/src/censor/src/Censor.ts index 09754540fc..8755d41925 100644 --- a/packages/imperative/src/censor/src/Censor.ts +++ b/packages/imperative/src/censor/src/Censor.ts @@ -193,7 +193,7 @@ export class Censor { * @returns {boolean} - True if the property is secure; False otherwise */ public static isSecureValue(prop: string) { - return this.SECURE_PROMPT_OPTIONS.includes(prop); + return this.CENSORED_OPTIONS.includes(prop); } /**************************************************************************************** diff --git a/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts b/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts index 6c8daabc27..53c472cdc5 100644 --- a/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts +++ b/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts @@ -1499,7 +1499,8 @@ describe("Command Processor", () => { } }, layers: [{ exists: true, path: "zowe.config.json" }], - properties: Config.empty() + properties: Config.empty(), + mProperties: Config.empty() } } as any); diff --git a/packages/imperative/src/logger/__tests__/LoggerUtils.unit.test.ts b/packages/imperative/src/logger/__tests__/LoggerUtils.unit.test.ts index 70c15247f9..ee36c52412 100644 --- a/packages/imperative/src/logger/__tests__/LoggerUtils.unit.test.ts +++ b/packages/imperative/src/logger/__tests__/LoggerUtils.unit.test.ts @@ -102,9 +102,10 @@ describe("LoggerUtils tests", () => { describe("special value:", () => { beforeEach(() => { - impConfig.config.mProperties = {profiles: {secret: { properties: {}}}}; + impConfig.config.mProperties = {profiles: {secret: { properties: {}}}, defaults: {}}; impConfigSpy.mockReturnValue(impConfig); envSettingsReadSpy.mockReturnValue({ maskOutput: { value: "TRUE" } }); + (Censor as any).addCensoredOption("secret"); }); const _lazyTest = (prop: string): [string, string] => { From 6e44d21fd0cc0965ca5ed0b34947aff79aca36f5 Mon Sep 17 00:00:00 2001 From: "Andrew W. Harn" Date: Thu, 6 Feb 2025 15:01:30 -0500 Subject: [PATCH 14/21] Get tests to pass Signed-off-by: Andrew W. Harn --- ...-test-cli.test.masking.integration.test.ts | 2 +- .../cli/test/masking/Masking.definition.ts | 5 ++- .../src/cli/test/masking/Masking.handler.ts | 32 +++++++-------- .../imperative/src/imperative.ts | 4 +- .../src/censor/__tests__/Censor.unit.test.ts | 41 ++++++++++++++----- 5 files changed, 53 insertions(+), 31 deletions(-) diff --git a/packages/imperative/__tests__/__integration__/imperative/__tests__/__integration__/cli/test/cli.imperative-test-cli.test.masking.integration.test.ts b/packages/imperative/__tests__/__integration__/imperative/__tests__/__integration__/cli/test/cli.imperative-test-cli.test.masking.integration.test.ts index 47b85b5b78..7fde9f87e4 100644 --- a/packages/imperative/__tests__/__integration__/imperative/__tests__/__integration__/cli/test/cli.imperative-test-cli.test.masking.integration.test.ts +++ b/packages/imperative/__tests__/__integration__/imperative/__tests__/__integration__/cli/test/cli.imperative-test-cli.test.masking.integration.test.ts @@ -36,7 +36,7 @@ describe("imperative-test-cli test masking command", () => { TestLogger.info(`Working directory: ${TEST_ENVIRONMENT.workingDir}`); runCliScript(__dirname + "/../config/init/__scripts__/init_config.sh", TEST_ENVIRONMENT.workingDir, ["--no-prompt"]); runCliScript(__dirname + "/../config/set/__scripts__/set_secure.sh", TEST_ENVIRONMENT.workingDir, - ["profiles.secured.profiles.secure_profile.properties.info", "secret"]); + ["profiles.secured.properties.info", "secret"]); }); const _logPrefix = (_log: string, level: string) => { diff --git a/packages/imperative/__tests__/__integration__/imperative/src/cli/test/masking/Masking.definition.ts b/packages/imperative/__tests__/__integration__/imperative/src/cli/test/masking/Masking.definition.ts index a2ec8e59bc..61e8b1f9e8 100644 --- a/packages/imperative/__tests__/__integration__/imperative/src/cli/test/masking/Masking.definition.ts +++ b/packages/imperative/__tests__/__integration__/imperative/src/cli/test/masking/Masking.definition.ts @@ -17,8 +17,11 @@ export const MaskingDefinition: ICommandDefinition = { summary: "Test imperative masking", type: "command", handler: __dirname + "/Masking.handler", + profile: { + optional: ["secured"] + }, positionals: [{ - name: "test-argument", + name: "info", description: "test argument", type: "string", required: true, diff --git a/packages/imperative/__tests__/__integration__/imperative/src/cli/test/masking/Masking.handler.ts b/packages/imperative/__tests__/__integration__/imperative/src/cli/test/masking/Masking.handler.ts index 9344f0ff69..fabf83cfab 100644 --- a/packages/imperative/__tests__/__integration__/imperative/src/cli/test/masking/Masking.handler.ts +++ b/packages/imperative/__tests__/__integration__/imperative/src/cli/test/masking/Masking.handler.ts @@ -19,25 +19,25 @@ import { IHandlerParameters, ICommandHandler, Imperative } from "../../../../../ export default class MaskingHandler implements ICommandHandler { public async process(params: IHandlerParameters) { // Issue various log messages to the console logger - params.response.console.log("Test-Masking: console logger: log message: " + params.arguments.testArgument); - params.response.console.error("Test-Masking: console logger: error message: " + params.arguments.testArgument); - params.response.console.errorHeader("Test-Masking: console logger: errorHeader message: " + params.arguments.testArgument); - params.response.console.prompt("Test-Masking: console logger: prompt message: " + params.arguments.testArgument); + params.response.console.log("Test-Masking: console logger: log message: " + params.arguments.info); + params.response.console.error("Test-Masking: console logger: error message: " + params.arguments.info); + params.response.console.errorHeader("Test-Masking: console logger: errorHeader message: " + params.arguments.info); + params.response.console.prompt("Test-Masking: console logger: prompt message: " + params.arguments.info); // Issue various log messages to the imperative logger - Imperative.api.imperativeLogger.trace("Test-Masking: imperative logger: trace message: " + params.arguments.testArgument); - Imperative.api.imperativeLogger.debug("Test-Masking: imperative logger: debug message: " + params.arguments.testArgument); - Imperative.api.imperativeLogger.info("Test-Masking: imperative logger: info message: " + params.arguments.testArgument); - Imperative.api.imperativeLogger.warn("Test-Masking: imperative logger: warn message: " + params.arguments.testArgument); - Imperative.api.imperativeLogger.error("Test-Masking: imperative logger: error message: " + params.arguments.testArgument); - Imperative.api.imperativeLogger.fatal("Test-Masking: imperative logger: fatal message: " + params.arguments.testArgument); + Imperative.api.imperativeLogger.trace("Test-Masking: imperative logger: trace message: " + params.arguments.info); + Imperative.api.imperativeLogger.debug("Test-Masking: imperative logger: debug message: " + params.arguments.info); + Imperative.api.imperativeLogger.info("Test-Masking: imperative logger: info message: " + params.arguments.info); + Imperative.api.imperativeLogger.warn("Test-Masking: imperative logger: warn message: " + params.arguments.info); + Imperative.api.imperativeLogger.error("Test-Masking: imperative logger: error message: " + params.arguments.info); + Imperative.api.imperativeLogger.fatal("Test-Masking: imperative logger: fatal message: " + params.arguments.info); // Issue various log messages to the app logger - Imperative.api.appLogger.trace("Test-Masking: app logger: trace message: " + params.arguments.testArgument); - Imperative.api.appLogger.debug("Test-Masking: app logger: debug message: " + params.arguments.testArgument); - Imperative.api.appLogger.info("Test-Masking: app logger: info message: " + params.arguments.testArgument); - Imperative.api.appLogger.warn("Test-Masking: app logger: warn message: " + params.arguments.testArgument); - Imperative.api.appLogger.error("Test-Masking: app logger: error message: " + params.arguments.testArgument); - Imperative.api.appLogger.fatal("Test-Masking: app logger: fatal message: " + params.arguments.testArgument); + Imperative.api.appLogger.trace("Test-Masking: app logger: trace message: " + params.arguments.info); + Imperative.api.appLogger.debug("Test-Masking: app logger: debug message: " + params.arguments.info); + Imperative.api.appLogger.info("Test-Masking: app logger: info message: " + params.arguments.info); + Imperative.api.appLogger.warn("Test-Masking: app logger: warn message: " + params.arguments.info); + Imperative.api.appLogger.error("Test-Masking: app logger: error message: " + params.arguments.info); + Imperative.api.appLogger.fatal("Test-Masking: app logger: fatal message: " + params.arguments.info); } } diff --git a/packages/imperative/__tests__/__integration__/imperative/src/imperative.ts b/packages/imperative/__tests__/__integration__/imperative/src/imperative.ts index 3c3f68e855..36b46c0872 100644 --- a/packages/imperative/__tests__/__integration__/imperative/src/imperative.ts +++ b/packages/imperative/__tests__/__integration__/imperative/src/imperative.ts @@ -106,12 +106,12 @@ export const config: IImperativeConfig = { info: { type: "string", includeInTemplate: true, - optionDefinition: {...infoOption, required: true} + optionDefinition: infoOption }, secret: { type: "string", secure: true, - optionDefinition: {...secretOption, required: true} + optionDefinition: secretOption } } } diff --git a/packages/imperative/src/censor/__tests__/Censor.unit.test.ts b/packages/imperative/src/censor/__tests__/Censor.unit.test.ts index ec083fd8e8..22e7175a41 100644 --- a/packages/imperative/src/censor/__tests__/Censor.unit.test.ts +++ b/packages/imperative/src/censor/__tests__/Censor.unit.test.ts @@ -528,6 +528,25 @@ describe("Censor tests", () => { expect(Censor.CENSORED_OPTIONS).toContain("test2"); expect(Censor.CENSORED_OPTIONS).toContain("t"); }); + + it("should add secure props with no option definitions to the secure array", () => { + const fakeSchema = { + type: "test", + schema: { + title: "Fake Profile Type", + description: "Fake Profile Description", + type: "object", + properties: { + test: { + type: "string", + secure: true + } + } + } + }; + (Censor as any).handleSchema(fakeSchema); + expect(Censor.CENSORED_OPTIONS).toContain("test"); + }); }); describe("setCensoredOptions", () => { @@ -613,7 +632,7 @@ describe("Censor tests", () => { required: ["test"] } }, - } + }; Censor.setCensoredOptions(censorOpts); @@ -656,7 +675,7 @@ describe("Censor tests", () => { optional: ["test"] } }, - } + }; Censor.setCensoredOptions(censorOpts); @@ -697,7 +716,7 @@ describe("Censor tests", () => { type: "command", profile: {} }, - } + }; Censor.setCensoredOptions(censorOpts); @@ -740,7 +759,7 @@ describe("Censor tests", () => { optional: ["nottest"] } }, - } + }; Censor.setCensoredOptions(censorOpts); @@ -767,7 +786,7 @@ describe("Censor tests", () => { } } } as any - } + }; Censor.setCensoredOptions(censorOpts); expect(Censor.CENSORED_OPTIONS).toContain("host"); @@ -785,7 +804,7 @@ describe("Censor tests", () => { "user" ] } - } + }; const censorOpts: ICensorOptions = { config: { api: { @@ -821,7 +840,7 @@ describe("Censor tests", () => { commandArguments: { "test-profile": "test1" } as any - } + }; Censor.setCensoredOptions(censorOpts); expect(Censor.CENSORED_OPTIONS).toContain("host"); @@ -839,7 +858,7 @@ describe("Censor tests", () => { "user" ] } - } + }; const censorOpts: ICensorOptions = { config: { api: { @@ -875,7 +894,7 @@ describe("Censor tests", () => { commandArguments: { "test-profile": "test1" } as any - } + }; Censor.setCensoredOptions(censorOpts); expect(Censor.CENSORED_OPTIONS).toContain("host"); @@ -893,7 +912,7 @@ describe("Censor tests", () => { "user" ] } - } + }; const censorOpts: ICensorOptions = { config: { api: { @@ -931,7 +950,7 @@ describe("Censor tests", () => { commandArguments: { "test-profile": "test1" } as any - } + }; Censor.setCensoredOptions(censorOpts); expect(Censor.CENSORED_OPTIONS).not.toContain("host"); From ea53f38787b57b5686c96b1b4c8f3c792607daff Mon Sep 17 00:00:00 2001 From: "Andrew W. Harn" Date: Thu, 6 Feb 2025 16:09:25 -0500 Subject: [PATCH 15/21] Make some code smells and add changelog Signed-off-by: Andrew W. Harn --- packages/imperative/CHANGELOG.md | 7 +++++++ packages/imperative/src/censor/src/Censor.ts | 2 +- packages/imperative/src/logger/src/LoggerUtils.ts | 4 ++-- .../src/rest/src/session/ConnectionPropsForSessCfg.ts | 2 +- 4 files changed, 11 insertions(+), 4 deletions(-) diff --git a/packages/imperative/CHANGELOG.md b/packages/imperative/CHANGELOG.md index 6d269b63d1..95130e83c5 100644 --- a/packages/imperative/CHANGELOG.md +++ b/packages/imperative/CHANGELOG.md @@ -3,7 +3,14 @@ All notable changes to the Imperative package will be documented in this file. ## Recent Changes + +- Enhancement: Added the `Censor` class, consolidating all sensitive data hiding logic into one class. [#2424](https://github.com/zowe/zowe-cli/pull/2424) - Format fix: `DeferredPromise` and `DeferredPromise.unit.test` comment format changed to match standard. +- Deprecated: `LoggerUtils` class has been deprecated. Use `Censor` class instead. [#2424](https://github.com/zowe/zowe-cli/pull/2424) +- Deprecated: `CliUtils.CENSOR_RESPONSE` has been deprecated. Use `Censor.CENSOR_RESPONSE` instead. [#2424](https://github.com/zowe/zowe-cli/pull/2424) +- Deprecated: `CliUtils.CENSORED_OPTIONS` has been deprecated. Use `Censor.CENSORED_OPTIONS` instead. [#2424](https://github.com/zowe/zowe-cli/pull/2424) +- Deprecated: `CliUtils.censorCLIArgs` has been deprecated. Use `Censor.censorCLIArgs` instead. [#2424](https://github.com/zowe/zowe-cli/pull/2424) +- Deprecated: `CliUtils.censorYargsArguments` has been deprecated. Use `Censor.censorYargsArguments` instead. [#2424](https://github.com/zowe/zowe-cli/pull/2424) ## `8.11.0` diff --git a/packages/imperative/src/censor/src/Censor.ts b/packages/imperative/src/censor/src/Censor.ts index 8755d41925..137aad5738 100644 --- a/packages/imperative/src/censor/src/Censor.ts +++ b/packages/imperative/src/censor/src/Censor.ts @@ -129,7 +129,7 @@ export class Censor { } } else if (prop.optionDefinitions) { // Handle the case of multiple option definitions - prop.optionDefinitions.map(opDef => { + prop.optionDefinitions.forEach(opDef => { secureOptions.add(opDef.name); for (const alias of opDef.aliases || []) { // Remember to add the alias diff --git a/packages/imperative/src/logger/src/LoggerUtils.ts b/packages/imperative/src/logger/src/LoggerUtils.ts index 6e27840429..81506489cd 100644 --- a/packages/imperative/src/logger/src/LoggerUtils.ts +++ b/packages/imperative/src/logger/src/LoggerUtils.ts @@ -22,7 +22,7 @@ export class LoggerUtils { /** * @deprecated Use Censor.CENSOR_RESPONSE */ - public static CENSOR_RESPONSE = Censor.CENSOR_RESPONSE; + public static readonly CENSOR_RESPONSE = Censor.CENSOR_RESPONSE; /** * @deprecated Use Censor.CENSORED_OPTIONS @@ -32,7 +32,7 @@ export class LoggerUtils { /** * @deprecated Use Censor.SECURE_PROMPT_OPTIONS */ - public static SECURE_PROMPT_OPTIONS = Censor.SECURE_PROMPT_OPTIONS; + public static readonly SECURE_PROMPT_OPTIONS = Censor.SECURE_PROMPT_OPTIONS; /** * Copy and censor any sensitive CLI arguments before logging/printing diff --git a/packages/imperative/src/rest/src/session/ConnectionPropsForSessCfg.ts b/packages/imperative/src/rest/src/session/ConnectionPropsForSessCfg.ts index 6bcbb3b24f..3766bd913d 100644 --- a/packages/imperative/src/rest/src/session/ConnectionPropsForSessCfg.ts +++ b/packages/imperative/src/rest/src/session/ConnectionPropsForSessCfg.ts @@ -355,7 +355,7 @@ export class ConnectionPropsForSessCfg { * List of properties on `sessCfg` object that should be kept secret and * may not appear in Imperative log files. */ - private static secureSessCfgProps: Set = new Set(Censor.SECURE_PROMPT_OPTIONS); + private static readonly secureSessCfgProps: Set = new Set(Censor.SECURE_PROMPT_OPTIONS); /** * List of prompt messages that is used when the CLI prompts for session From 292588405d535f2c8e18aaea2258f7bf0ffaf90f Mon Sep 17 00:00:00 2001 From: "Andrew W. Harn" Date: Thu, 6 Feb 2025 16:10:05 -0500 Subject: [PATCH 16/21] Small changelog tweak Signed-off-by: Andrew W. Harn --- packages/imperative/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/imperative/CHANGELOG.md b/packages/imperative/CHANGELOG.md index 95130e83c5..f538d4e58c 100644 --- a/packages/imperative/CHANGELOG.md +++ b/packages/imperative/CHANGELOG.md @@ -6,7 +6,7 @@ All notable changes to the Imperative package will be documented in this file. - Enhancement: Added the `Censor` class, consolidating all sensitive data hiding logic into one class. [#2424](https://github.com/zowe/zowe-cli/pull/2424) - Format fix: `DeferredPromise` and `DeferredPromise.unit.test` comment format changed to match standard. -- Deprecated: `LoggerUtils` class has been deprecated. Use `Censor` class instead. [#2424](https://github.com/zowe/zowe-cli/pull/2424) +- Deprecated: The `LoggerUtils` class has been deprecated. Use the `Censor` class instead. [#2424](https://github.com/zowe/zowe-cli/pull/2424) - Deprecated: `CliUtils.CENSOR_RESPONSE` has been deprecated. Use `Censor.CENSOR_RESPONSE` instead. [#2424](https://github.com/zowe/zowe-cli/pull/2424) - Deprecated: `CliUtils.CENSORED_OPTIONS` has been deprecated. Use `Censor.CENSORED_OPTIONS` instead. [#2424](https://github.com/zowe/zowe-cli/pull/2424) - Deprecated: `CliUtils.censorCLIArgs` has been deprecated. Use `Censor.censorCLIArgs` instead. [#2424](https://github.com/zowe/zowe-cli/pull/2424) From 1f427d1ffa2a2a5f6b8151c10265ef17289676b2 Mon Sep 17 00:00:00 2001 From: "Andrew W. Harn" Date: Fri, 7 Feb 2025 10:18:37 -0500 Subject: [PATCH 17/21] Fix #2430, move environment variable, and update tests Signed-off-by: Andrew W. Harn --- packages/imperative/CHANGELOG.md | 2 ++ ...lVariableSettings.integration.test.ts.snap | 4 +++ .../src/censor/__tests__/Censor.unit.test.ts | 26 +++++++++++++++++-- packages/imperative/src/censor/src/Censor.ts | 6 ++++- .../__tests__/CommandProcessor.unit.test.ts | 3 ++- .../src/cmd/src/CommandProcessor.ts | 12 +++------ .../imperative/src/constants/src/Constants.ts | 1 + ...EnvironmentalVariableSettings.unit.test.ts | 3 ++- ...ImperativeEnvironmentalVariableSettings.ts | 5 ++++ .../src/env/EnvironmentalVariableSettings.ts | 11 +++++++- .../EnvironmentalVariableSettings.ts | 2 ++ 11 files changed, 60 insertions(+), 15 deletions(-) diff --git a/packages/imperative/CHANGELOG.md b/packages/imperative/CHANGELOG.md index f538d4e58c..5d2f225cb4 100644 --- a/packages/imperative/CHANGELOG.md +++ b/packages/imperative/CHANGELOG.md @@ -4,7 +4,9 @@ All notable changes to the Imperative package will be documented in this file. ## Recent Changes +- BugFix: Fixed inconsistent behavior with the `ZOWE_SHOW_SECURE_ARGS` environment variable continuing to mask secure properties when it should not. [#2430](https://github.com/zowe/zowe-cli/issues/2430) - Enhancement: Added the `Censor` class, consolidating all sensitive data hiding logic into one class. [#2424](https://github.com/zowe/zowe-cli/pull/2424) +- Enhancement: Added `showSecureArgs` to `EnvironmentalVariableSettings` to allow extenders to determine if they should mask secure values. [#2424](https://github.com/zowe/zowe-cli/pull/2424) - Format fix: `DeferredPromise` and `DeferredPromise.unit.test` comment format changed to match standard. - Deprecated: The `LoggerUtils` class has been deprecated. Use the `Censor` class instead. [#2424](https://github.com/zowe/zowe-cli/pull/2424) - Deprecated: `CliUtils.CENSOR_RESPONSE` has been deprecated. Use `Censor.CENSOR_RESPONSE` instead. [#2424](https://github.com/zowe/zowe-cli/pull/2424) diff --git a/packages/imperative/__tests__/__integration__/imperative/__tests__/__integration__/api/env/__snapshots__/EnvironmentalVariableSettings.integration.test.ts.snap b/packages/imperative/__tests__/__integration__/imperative/__tests__/__integration__/api/env/__snapshots__/EnvironmentalVariableSettings.integration.test.ts.snap index 923ac4ed23..fabb842699 100644 --- a/packages/imperative/__tests__/__integration__/imperative/__tests__/__integration__/api/env/__snapshots__/EnvironmentalVariableSettings.integration.test.ts.snap +++ b/packages/imperative/__tests__/__integration__/imperative/__tests__/__integration__/api/env/__snapshots__/EnvironmentalVariableSettings.integration.test.ts.snap @@ -26,5 +26,9 @@ Object { "key": "IMP_INTEGRATION_TESTING_PROMPT_PHRASE", "value": undefined, }, + "showSecureArgs": Object { + "key": "IMP_INTEGRATION_TESTING_SHOW_SECURE_ARGS", + "value": "FALSE", + }, } `; diff --git a/packages/imperative/src/censor/__tests__/Censor.unit.test.ts b/packages/imperative/src/censor/__tests__/Censor.unit.test.ts index 22e7175a41..a4f2af5162 100644 --- a/packages/imperative/src/censor/__tests__/Censor.unit.test.ts +++ b/packages/imperative/src/censor/__tests__/Censor.unit.test.ts @@ -133,7 +133,7 @@ describe("Censor tests", () => { envVariablePrefix: "ZOWE" }); findSecure.mockReturnValue([]); - envSettingsReadSpy.mockReturnValue({ maskOutput: { value: "FALSE" } }); + envSettingsReadSpy.mockReturnValue({ maskOutput: { value: "FALSE" }, showSecureArgs: { value: "FALSE" } }); expect(Censor.censorRawData(secrets[1], "console")).toEqual(secrets[1]); }); @@ -143,10 +143,32 @@ describe("Censor tests", () => { envVariablePrefix: "ZOWE" }); findSecure.mockReturnValue([]); - envSettingsReadSpy.mockReturnValue({ maskOutput: { value: "FALSE" } }); + envSettingsReadSpy.mockReturnValue({ maskOutput: { value: "FALSE" }, showSecureArgs: { value: "FALSE" } }); expect(Censor.censorRawData(secrets[1], "json")).toEqual(secrets[1]); }); + it("Console Output if the SHOW_SECURE_ARGS env var is FALSE and category is console", () => { + impConfigSpy.mockReturnValue({ + config: { exists: true, api: { secure: { findSecure }}, mProperties: { profiles: []}}, + envVariablePrefix: "ZOWE" + }); + findSecure.mockReturnValue([]); + envSettingsReadSpy.mockReturnValue({ maskOutput: { value: "TRUE" }, showSecureArgs: { value: "TRUE"}}); + const censoredData = Censor.censorRawData(secrets[1], "console"); + expect(censoredData).toEqual(secrets[1]); + }); + + it("Console Output if the SHOW_SECURE_ARGS env var is FALSE and category is json", () => { + impConfigSpy.mockReturnValue({ + config: { exists: true, api: { secure: { findSecure }}, mProperties: { profiles: []}}, + envVariablePrefix: "ZOWE" + }); + findSecure.mockReturnValue([]); + envSettingsReadSpy.mockReturnValue({ maskOutput: { value: "TRUE" }, showSecureArgs: { value: "TRUE" } }); + const censoredData = Censor.censorRawData(secrets[1], "json"); + expect(censoredData).toEqual(secrets[1]); + }); + describe("special value:", () => { beforeEach(() => { impConfig.config.mProperties = {profiles: {secret: { properties: {}}}, defaults: {}}; diff --git a/packages/imperative/src/censor/src/Censor.ts b/packages/imperative/src/censor/src/Censor.ts index 137aad5738..c066493bd2 100644 --- a/packages/imperative/src/censor/src/Censor.ts +++ b/packages/imperative/src/censor/src/Censor.ts @@ -296,8 +296,12 @@ export class Censor { // Return the data if we are printing to the console and masking is disabled if (ImperativeConfig.instance?.envVariablePrefix) { const envMaskOutput = EnvironmentalVariableSettings.read(ImperativeConfig.instance.envVariablePrefix).maskOutput.value; + const envShowSecureArgs = EnvironmentalVariableSettings.read(ImperativeConfig.instance.envVariablePrefix).showSecureArgs.value; // Hardcoding "console" instead of using Logger.DEFAULT_CONSOLE_NAME because of circular dependencies - if ((category === "console" || category === "json") && envMaskOutput.toUpperCase() === "FALSE") return data; + if ((category === "console" || category === "json") && + (envMaskOutput.toUpperCase() === "FALSE" || envShowSecureArgs.toUpperCase() === "TRUE")) { + return data; + } } let newData = data; diff --git a/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts b/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts index 53c472cdc5..f86d477686 100644 --- a/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts +++ b/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts @@ -1501,7 +1501,8 @@ describe("Command Processor", () => { layers: [{ exists: true, path: "zowe.config.json" }], properties: Config.empty(), mProperties: Config.empty() - } + }, + envVariablePrefix: "ZOWE" } as any); // Allocate the command processor diff --git a/packages/imperative/src/cmd/src/CommandProcessor.ts b/packages/imperative/src/cmd/src/CommandProcessor.ts index f7dd6ab998..6ab46bfe3e 100644 --- a/packages/imperative/src/cmd/src/CommandProcessor.ts +++ b/packages/imperative/src/cmd/src/CommandProcessor.ts @@ -43,6 +43,7 @@ import { ConfigConstants } from "../../config/src/ConfigConstants"; import { IDaemonContext } from "../../imperative/src/doc/IDaemonContext"; import { IHandlerResponseApi } from "./doc/response/api/handler/IHandlerResponseApi"; import { Censor } from "../../censor/src/Censor"; +import { EnvironmentalVariableSettings } from "../../imperative/src/env/EnvironmentalVariableSettings"; /** @@ -87,13 +88,6 @@ interface IResolvedArgsResponse { * @class CommandProcessor */ export class CommandProcessor { - /** - * Show secure fields in the output of the command ENV var suffix - * @private - * @static - * @memberof CommandProcessor - */ - private static readonly ENV_SHOW_SECURE_SUFFIX = `_SHOW_SECURE_ARGS`; /** * The error tag for imperative errors. * @private @@ -733,8 +727,8 @@ export class CommandProcessor { let showSecure = false; // ZOWE_SHOW_SECURE_ARGS - const SECRETS_ENV = `${ImperativeConfig.instance.envVariablePrefix}${CommandProcessor.ENV_SHOW_SECURE_SUFFIX}`; - const env = (process.env[SECRETS_ENV] || "false").toUpperCase(); + const SECRETS_ENV = `${ImperativeConfig.instance.envVariablePrefix}${EnvironmentalVariableSettings.ENV_SHOW_SECURE_SUFFIX}`; + const env = EnvironmentalVariableSettings.read(ImperativeConfig.instance.envVariablePrefix).showSecureArgs.value.toUpperCase(); if (env === "TRUE" || env === "1") { showSecure = true; diff --git a/packages/imperative/src/constants/src/Constants.ts b/packages/imperative/src/constants/src/Constants.ts index f247b56acb..937d772a73 100644 --- a/packages/imperative/src/constants/src/Constants.ts +++ b/packages/imperative/src/constants/src/Constants.ts @@ -104,6 +104,7 @@ export class Constants { public static readonly DEFAULT_HIGHLIGHT_COLOR = "yellow"; public static readonly DEFAULT_PROMPT_PHRASE = "PROMPT*"; public static readonly DEFAULT_MASK_OUTPUT = "TRUE"; + public static readonly DEFAULT_SHOW_SECURE = "FALSE"; public static readonly WEB_HELP_DIR = "web-help"; public static readonly WEB_DIFF_DIR = "web-diff"; diff --git a/packages/imperative/src/imperative/__tests__/env/EnvironmentalVariableSettings.unit.test.ts b/packages/imperative/src/imperative/__tests__/env/EnvironmentalVariableSettings.unit.test.ts index 0f2e479411..5bf312cf49 100644 --- a/packages/imperative/src/imperative/__tests__/env/EnvironmentalVariableSettings.unit.test.ts +++ b/packages/imperative/src/imperative/__tests__/env/EnvironmentalVariableSettings.unit.test.ts @@ -30,7 +30,8 @@ describe("EnvironmentalVariableSettings tests", () => { cliHome: getSetting(prefix + EnvironmentalVariableSettings.CLI_HOME_SUFFIX), promptPhrase: getSetting(prefix + EnvironmentalVariableSettings.PROMPT_PHRASE_SUFFIX), maskOutput: getSetting(prefix + EnvironmentalVariableSettings.APP_MASK_OUTPUT_SUFFIX, Constants.DEFAULT_MASK_OUTPUT), - pluginsDir: getSetting(prefix + EnvironmentalVariableSettings.CLI_PLUGINS_DIR_SUFFIX) + pluginsDir: getSetting(prefix + EnvironmentalVariableSettings.CLI_PLUGINS_DIR_SUFFIX), + showSecureArgs: getSetting(prefix + EnvironmentalVariableSettings.ENV_SHOW_SECURE_SUFFIX, Constants.DEFAULT_SHOW_SECURE), }; }; diff --git a/packages/imperative/src/imperative/src/doc/IImperativeEnvironmentalVariableSettings.ts b/packages/imperative/src/imperative/src/doc/IImperativeEnvironmentalVariableSettings.ts index 628954a3ef..6df46897db 100644 --- a/packages/imperative/src/imperative/src/doc/IImperativeEnvironmentalVariableSettings.ts +++ b/packages/imperative/src/imperative/src/doc/IImperativeEnvironmentalVariableSettings.ts @@ -53,4 +53,9 @@ export interface IImperativeEnvironmentalVariableSettings { * Default is `${cliHome}/plugins`. */ pluginsDir?: IImperativeEnvironmentalVariableSetting; + + /** + * Whether or not to show secure args. + */ + showSecureArgs?: IImperativeEnvironmentalVariableSetting; } diff --git a/packages/imperative/src/imperative/src/env/EnvironmentalVariableSettings.ts b/packages/imperative/src/imperative/src/env/EnvironmentalVariableSettings.ts index 8bd0601e00..d5ff2ff523 100644 --- a/packages/imperative/src/imperative/src/env/EnvironmentalVariableSettings.ts +++ b/packages/imperative/src/imperative/src/env/EnvironmentalVariableSettings.ts @@ -69,6 +69,13 @@ export class EnvironmentalVariableSettings { */ public static readonly CLI_PLUGINS_DIR_SUFFIX = "_CLI_PLUGINS_DIR"; + /** + * Show secure fields in the output of the command ENV var suffix + * @type {string} + * @memberof EnvironmentalVariableSettings + */ + public static readonly ENV_SHOW_SECURE_SUFFIX = `_SHOW_SECURE_ARGS`; + /** * Read all environmental variable settings for a CLI @@ -97,7 +104,9 @@ export class EnvironmentalVariableSettings { maskOutput: getSetting(prefix + this.APP_MASK_OUTPUT_SUFFIX, Constants.DEFAULT_MASK_OUTPUT), pluginsDir: - getSetting(prefix + this.CLI_PLUGINS_DIR_SUFFIX) + getSetting(prefix + this.CLI_PLUGINS_DIR_SUFFIX), + showSecureArgs: + getSetting(prefix + this.ENV_SHOW_SECURE_SUFFIX, Constants.DEFAULT_SHOW_SECURE), }; } } diff --git a/packages/imperative/src/imperative/src/env/__mocks__/EnvironmentalVariableSettings.ts b/packages/imperative/src/imperative/src/env/__mocks__/EnvironmentalVariableSettings.ts index 66345d3a32..adabedc52f 100644 --- a/packages/imperative/src/imperative/src/env/__mocks__/EnvironmentalVariableSettings.ts +++ b/packages/imperative/src/imperative/src/env/__mocks__/EnvironmentalVariableSettings.ts @@ -22,6 +22,7 @@ const EnvironmentalVariableSettings: any = (EnvironmentalVariableSettings.CLI_HOME_SUFFIX as any) = envActual.CLI_HOME_SUFFIX; (EnvironmentalVariableSettings.PROMPT_PHRASE_SUFFIX as any) = envActual.PROMPT_PHRASE_SUFFIX; (EnvironmentalVariableSettings.APP_MASK_OUTPUT_SUFFIX as any) = envActual.APP_MASK_OUTPUT_SUFFIX; +(EnvironmentalVariableSettings.ENV_SHOW_SECURE_SUFFIX as any) = envActual.ENV_SHOW_SECURE_SUFFIX; (EnvironmentalVariableSettings.read as Mock).mockImplementation((prefix: string): IImperativeEnvironmentalVariableSettings => { const getSetting = (key: string) => { @@ -34,6 +35,7 @@ const EnvironmentalVariableSettings: any = cliHome: getSetting(EnvironmentalVariableSettings.CLI_HOME_SUFFIX), promptPhrase: getSetting(EnvironmentalVariableSettings.PROMPT_PHRASE_SUFFIX), maskOutput: getSetting(EnvironmentalVariableSettings.APP_MASK_OUTPUT_SUFFIX), + showSecureArgs: getSetting(EnvironmentalVariableSettings.ENV_SHOW_SECURE_SUFFIX), }; }); From 7f371238a947cf0531aab23321f2f44b38670802 Mon Sep 17 00:00:00 2001 From: "Andrew W. Harn" Date: Mon, 10 Feb 2025 09:23:04 -0500 Subject: [PATCH 18/21] Fix broken Copy system tests Signed-off-by: Andrew W. Harn --- .../__tests__/__system__/methods/copy/Copy.system.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/zosfiles/__tests__/__system__/methods/copy/Copy.system.test.ts b/packages/zosfiles/__tests__/__system__/methods/copy/Copy.system.test.ts index 453a9ea0df..631ac74036 100644 --- a/packages/zosfiles/__tests__/__system__/methods/copy/Copy.system.test.ts +++ b/packages/zosfiles/__tests__/__system__/methods/copy/Copy.system.test.ts @@ -22,7 +22,8 @@ import { ITestEnvironment } from "../../../../../../__tests__/__src__/environmen import { tmpdir } from "os"; import path = require("path"); import * as fs from "fs"; -import { ZosmfRestClient, List } from "@zowe/core-for-zowe-sdk"; +import { ZosmfRestClient } from "@zowe/core-for-zowe-sdk"; +import { List } from "../../../../src/methods/list/List"; let REAL_SESSION: Session; From 83f29aaf156ddaf0ef936d50cb83e23d0b8d9e3e Mon Sep 17 00:00:00 2001 From: "Andrew W. Harn" Date: Mon, 10 Feb 2025 09:33:41 -0500 Subject: [PATCH 19/21] Combine imports Signed-off-by: Andrew W. Harn --- .../__tests__/__system__/methods/copy/Copy.system.test.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/zosfiles/__tests__/__system__/methods/copy/Copy.system.test.ts b/packages/zosfiles/__tests__/__system__/methods/copy/Copy.system.test.ts index 631ac74036..6a8e9ffc5a 100644 --- a/packages/zosfiles/__tests__/__system__/methods/copy/Copy.system.test.ts +++ b/packages/zosfiles/__tests__/__system__/methods/copy/Copy.system.test.ts @@ -11,7 +11,7 @@ import { Create, Upload, Delete, CreateDataSetTypeEnum, Copy, ZosFilesMessages, Get, IDataSet, ICrossLparCopyDatasetOptions, IGetOptions, IZosFilesResponse, - ZosFilesUtils} from "../../../../src"; + ZosFilesUtils, List } from "../../../../src"; import { Imperative, IO, Session } from "@zowe/imperative"; import { inspect } from "util"; import { TestEnvironment } from "../../../../../../__tests__/__src__/environment/TestEnvironment"; @@ -23,7 +23,6 @@ import { tmpdir } from "os"; import path = require("path"); import * as fs from "fs"; import { ZosmfRestClient } from "@zowe/core-for-zowe-sdk"; -import { List } from "../../../../src/methods/list/List"; let REAL_SESSION: Session; From 8b713e8054b0250d675d7308c2459eaa15f2a7b8 Mon Sep 17 00:00:00 2001 From: "Andrew W. Harn" Date: Mon, 10 Feb 2025 11:05:50 -0500 Subject: [PATCH 20/21] Update changelog Signed-off-by: Andrew W. Harn --- packages/imperative/CHANGELOG.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/imperative/CHANGELOG.md b/packages/imperative/CHANGELOG.md index 7bfdab17ac..9481d7b98e 100644 --- a/packages/imperative/CHANGELOG.md +++ b/packages/imperative/CHANGELOG.md @@ -6,12 +6,12 @@ All notable changes to the Imperative package will be documented in this file. - BugFix: Fixed inconsistent behavior with the `ZOWE_SHOW_SECURE_ARGS` environment variable continuing to mask secure properties when it should not. [#2430](https://github.com/zowe/zowe-cli/issues/2430) - Enhancement: Added the `Censor` class, consolidating all sensitive data hiding logic into one class. [#2424](https://github.com/zowe/zowe-cli/pull/2424) -- Enhancement: Added `showSecureArgs` to `EnvironmentalVariableSettings` to allow extenders to determine if they should mask secure values. [#2424](https://github.com/zowe/zowe-cli/pull/2424) +- Enhancement: Added the `showSecureArgs` environment variable to the `EnvironmentalVariableSettings` class to allow extenders to determine if they should mask secure values. [#2424](https://github.com/zowe/zowe-cli/pull/2424) - Deprecated: The `LoggerUtils` class has been deprecated. Use the `Censor` class instead. [#2424](https://github.com/zowe/zowe-cli/pull/2424) -- Deprecated: `CliUtils.CENSOR_RESPONSE` has been deprecated. Use `Censor.CENSOR_RESPONSE` instead. [#2424](https://github.com/zowe/zowe-cli/pull/2424) -- Deprecated: `CliUtils.CENSORED_OPTIONS` has been deprecated. Use `Censor.CENSORED_OPTIONS` instead. [#2424](https://github.com/zowe/zowe-cli/pull/2424) -- Deprecated: `CliUtils.censorCLIArgs` has been deprecated. Use `Censor.censorCLIArgs` instead. [#2424](https://github.com/zowe/zowe-cli/pull/2424) -- Deprecated: `CliUtils.censorYargsArguments` has been deprecated. Use `Censor.censorYargsArguments` instead. [#2424](https://github.com/zowe/zowe-cli/pull/2424) +- Deprecated: The `CliUtils.CENSOR_RESPONSE` property has been deprecated. Use the `Censor.CENSOR_RESPONSE` property instead. [#2424](https://github.com/zowe/zowe-cli/pull/2424) +- Deprecated: The `CliUtils.CENSORED_OPTIONS` property has been deprecated. Use the `Censor.CENSORED_OPTIONS` property instead. [#2424](https://github.com/zowe/zowe-cli/pull/2424) +- Deprecated: The `CliUtils.censorCLIArgs` function has been deprecated. Use the `Censor.censorCLIArgs` function instead. [#2424](https://github.com/zowe/zowe-cli/pull/2424) +- Deprecated: The `CliUtils.censorYargsArguments` function has been deprecated. Use the `Censor.censorYargsArguments` function instead. [#2424](https://github.com/zowe/zowe-cli/pull/2424) ## `8.13.0` From 348b65d207318ed42bc3c25ee723ce3d0aa012d6 Mon Sep 17 00:00:00 2001 From: "Andrew W. Harn" Date: Fri, 14 Feb 2025 12:03:17 -0500 Subject: [PATCH 21/21] Make most of Trae's requested changes Signed-off-by: Andrew W. Harn --- packages/imperative/src/censor/src/Censor.ts | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/packages/imperative/src/censor/src/Censor.ts b/packages/imperative/src/censor/src/Censor.ts index c066493bd2..0ae575ab8c 100644 --- a/packages/imperative/src/censor/src/Censor.ts +++ b/packages/imperative/src/censor/src/Censor.ts @@ -62,11 +62,11 @@ export class Censor { } // Set a censored options list that can be set and retrieved for each command. - private static censored_options: Set = new Set(this.DEFAULT_CENSORED_OPTIONS); + private static mCensoredOptions: Set = new Set(this.DEFAULT_CENSORED_OPTIONS); // Return a customized list of censored options (or just the defaults if not set). public static get CENSORED_OPTIONS(): string[] { - return Array.from(this.censored_options); + return Array.from(this.mCensoredOptions); } //Singleton caches of the Config, Command Definition and Command Arguments @@ -152,9 +152,9 @@ export class Censor { private static addCensoredOption(option: string) { // This option is required, but we do not want to ever allow null or undefined itself into the censored options if (option != null) { - this.censored_options.add(option); - this.censored_options.add(CliUtils.getOptionFormat(option).camelCase); - this.censored_options.add(CliUtils.getOptionFormat(option).kebabCase); + this.mCensoredOptions.add(option); + this.mCensoredOptions.add(CliUtils.getOptionFormat(option).camelCase); + this.mCensoredOptions.add(CliUtils.getOptionFormat(option).kebabCase); } } @@ -175,16 +175,12 @@ export class Censor { }; for (const profile of this.profileSchemas) { - // eslint-disable-next-line unused-imports/no-unused-vars - for (const [_, prop] of Object.entries(profile.schema.properties)) { + for (const prop of Object.values(profile.schema.properties)) { if (prop.secure) specialValues = lodash.union(specialValues, getPropertyNames(prop)); } } - for (const v of specialValues) { - if (prop.endsWith(`.${v}`)) return true; - } - return false; + return specialValues.some((v) => prop.endsWith(`.${v}`)); } /** @@ -206,7 +202,7 @@ export class Censor { * @param {ICensorOptions} censorOpts - The objects to use to gather options that should be censored */ public static setCensoredOptions(censorOpts?: ICensorOptions) { - this.censored_options = new Set(this.DEFAULT_CENSORED_OPTIONS); + this.mCensoredOptions = new Set(this.DEFAULT_CENSORED_OPTIONS); if (censorOpts) { // Save off the config object