Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add param noSensitiveData to Plugin.load #665

Merged
merged 7 commits into from
Mar 22, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions src/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -813,11 +813,11 @@ export class Config implements IConfig {
}

// when no manifest exists, the default is calculated. This may throw, so we need to catch it
const defaultFlagToCached = async (flag: CompletableOptionFlag<any>) => {
const defaultFlagToCached = async (flag: CompletableOptionFlag<any>, noSensitiveData = false) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add noSensitiveData param to defaultFlagToCached. Value is pass to defaultHelp and default

// Prefer the helpDefaultValue function (returns a friendly string for complex types)
if (typeof flag.defaultHelp === 'function') {
try {
return await flag.defaultHelp()
return await flag.defaultHelp({options: flag, flags: {}}, noSensitiveData)
} catch {
return
}
Expand All @@ -826,18 +826,18 @@ const defaultFlagToCached = async (flag: CompletableOptionFlag<any>) => {
// if not specified, try the default function
if (typeof flag.default === 'function') {
try {
return await flag.default({options: {}, flags: {}})
return await flag.default({options: {}, flags: {}}, noSensitiveData)
} catch {}
} else {
return flag.default
}
}

const defaultArgToCached = async (arg: Arg<any>): Promise<any> => {
const defaultArgToCached = async (arg: Arg<any>, noSensitiveData = false): Promise<any> => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add noSensitiveData param to defaultArgToCached. Value is pass to defaultHelp and default

// Prefer the helpDefaultValue function (returns a friendly string for complex types)
if (typeof arg.defaultHelp === 'function') {
try {
return await arg.defaultHelp()
return await arg.defaultHelp({options: arg, flags: {}}, noSensitiveData)
} catch {
return
}
Expand All @@ -846,14 +846,14 @@ const defaultArgToCached = async (arg: Arg<any>): Promise<any> => {
// if not specified, try the default function
if (typeof arg.default === 'function') {
try {
return await arg.default({options: {}, flags: {}})
return await arg.default({options: arg, flags: {}}, noSensitiveData)
} catch {}
} else {
return arg.default
}
}

export async function toCached(c: Command.Class, plugin?: IPlugin): Promise<Command.Cached> {
export async function toCached(c: Command.Class, plugin?: IPlugin | undefined, noSensitiveData?: boolean): Promise<Command.Cached> {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add noSensitiveData param to toCached. Value will be passes to defaultFlagToCached and defaultArgToCached functions

const flags = {} as {[k: string]: Command.Flag.Cached}

for (const [name, flag] of Object.entries(c.flags || {})) {
Expand Down Expand Up @@ -894,7 +894,7 @@ export async function toCached(c: Command.Class, plugin?: IPlugin): Promise<Comm
dependsOn: flag.dependsOn,
relationships: flag.relationships,
exclusive: flag.exclusive,
default: await defaultFlagToCached(flag),
default: await defaultFlagToCached(flag, noSensitiveData),
deprecated: flag.deprecated,
deprecateAliases: c.deprecateAliases,
aliases: flag.aliases,
Expand All @@ -914,7 +914,7 @@ export async function toCached(c: Command.Class, plugin?: IPlugin): Promise<Comm
description: arg.description,
required: arg.required,
options: arg.options,
default: await defaultArgToCached(arg),
default: await defaultArgToCached(arg, noSensitiveData),
hidden: arg.hidden,
}
}
Expand Down
8 changes: 4 additions & 4 deletions src/config/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ export class Plugin implements IPlugin {

constructor(public options: PluginOptions) {}

public async load(): Promise<void> {
public async load(noSensitiveData?: boolean): Promise<void> {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add noSensitiveData param to load. Value is passed to _manifest

this.type = this.options.type || 'core'
this.tag = this.options.tag
const root = await findRoot(this.options.name, this.options.root)
Expand All @@ -160,7 +160,7 @@ export class Plugin implements IPlugin {

this.hooks = mapValues(this.pjson.oclif.hooks || {}, i => Array.isArray(i) ? i : [i])

this.manifest = await this._manifest(Boolean(this.options.ignoreManifest), Boolean(this.options.errorOnManifestCreate))
this.manifest = await this._manifest(Boolean(this.options.ignoreManifest), Boolean(this.options.errorOnManifestCreate), noSensitiveData)
this.commands = Object
.entries(this.manifest.commands)
.map(([id, c]) => ({
Expand Down Expand Up @@ -244,7 +244,7 @@ export class Plugin implements IPlugin {
return cmd
}

protected async _manifest(ignoreManifest: boolean, errorOnManifestCreate = false): Promise<Manifest> {
protected async _manifest(ignoreManifest: boolean, errorOnManifestCreate = false, noSensitiveData = false): Promise<Manifest> {
const readManifest = async (dotfile = false): Promise<Manifest | undefined> => {
try {
const p = path.join(this.root, `${dotfile ? '.' : ''}oclif.manifest.json`)
Expand Down Expand Up @@ -279,7 +279,7 @@ export class Plugin implements IPlugin {
version: this.version,
commands: (await Promise.all(this.commandIDs.map(async id => {
try {
return [id, await toCached(await this.findCommand(id, {must: true}), this)]
return [id, await toCached(await this.findCommand(id, {must: true}), this, noSensitiveData)]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pass noSensitiveData to toCached

} catch (error: any) {
const scope = 'toCached'
if (Boolean(errorOnManifestCreate) === false) this.warn(error, scope)
Expand Down
2 changes: 1 addition & 1 deletion src/help/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ export class Help extends HelpBase {
const command = this.config.findCommand(subject)
if (command) {
if (command.hasDynamicHelp && command.pluginType !== 'jit') {
const dynamicCommand = await toCached(await command.load())
const dynamicCommand = await toCached(await command.load(), undefined, false)
await this.showCommandHelp(dynamicCommand)
} else {
await this.showCommandHelp(command)
Expand Down
10 changes: 5 additions & 5 deletions src/interfaces/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,11 @@ export type DefaultContext<T> = {
flags: Record<string, string>;
}

export type FlagDefault<T, P = CustomOptions> = T | ((context: DefaultContext<P & OptionFlag<T, P>>) => Promise<T>)
export type FlagDefaultHelp<T, P = CustomOptions> = T | ((context: DefaultContext<P & OptionFlag<T, P>>) => Promise<string | undefined>)
export type FlagDefault<T, P = CustomOptions> = T | ((context: DefaultContext<P & OptionFlag<T, P>>, noSensitiveData?: boolean) => Promise<T>)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add noSensitiveData to flag and arg default function types

export type FlagDefaultHelp<T, P = CustomOptions> = T | ((context: DefaultContext<P & OptionFlag<T, P>>, noSensitiveData?: boolean) => Promise<string | undefined>)

export type ArgDefault<T, P = CustomOptions> = T | ((context: DefaultContext<Arg<T, P>>) => Promise<T>)
export type ArgDefaultHelp<T, P = CustomOptions> = T | ((context: DefaultContext<Arg<T, P>>) => Promise<string | undefined>)
export type ArgDefault<T, P = CustomOptions> = T | ((context: DefaultContext<Arg<T, P>>, noSensitiveData?: boolean) => Promise<T>)
export type ArgDefaultHelp<T, P = CustomOptions> = T | ((context: DefaultContext<Arg<T, P>>, noSensitiveData?: boolean) => Promise<string | undefined>)

export type FlagRelationship = string | {name: string; when: (flags: Record<string, unknown>) => Promise<boolean>};
export type Relationship = {
Expand Down Expand Up @@ -208,7 +208,7 @@ export type BooleanFlag<T> = FlagProps & BooleanFlagProps & {

export type OptionFlagDefaults<T, P = CustomOptions, M = false> = FlagProps & OptionFlagProps & {
parse: FlagParser<T, string, P>
defaultHelp?: FlagDefaultHelp<T>;
defaultHelp?: T | FlagDefaultHelp<T>;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to check this change. Should not be needed

input: string[];
default?: M extends true ? FlagDefault<T[] | undefined, P> : FlagDefault<T | undefined, P>;
}
Expand Down
2 changes: 1 addition & 1 deletion src/interfaces/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,5 +73,5 @@ export interface Plugin {

findCommand(id: string, opts: { must: true }): Promise<Command.Class>;
findCommand(id: string, opts?: { must: boolean }): Promise<Command.Class> | undefined;
load(): Promise<void>;
load(noSensitiveData: boolean): Promise<void>;
}
2 changes: 1 addition & 1 deletion test/command/command.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ describe('command', () => {
}
}

const c = await toCached(C)
const c = await toCached(C, undefined, false)

expect(c).to.deep.equal({
id: 'foo:bar',
Expand Down
2 changes: 1 addition & 1 deletion test/help/help-test-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export class TestHelp extends Help {

export const commandHelp = (command?: any) => ({
async run(ctx: {help: TestHelp; commandHelp: string; expectation: string}) {
const cached = await toCached(command!, {} as any)
const cached = await toCached(command!, {} as any, false)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Command help is being called, do not suppress sensitive data

const help = ctx.help.formatCommand(cached)
if (process.env.TEST_OUTPUT === '1') {
console.log(help)
Expand Down