From bf1b41bdddff6b10456d1a167c0cf92de1bd9f84 Mon Sep 17 00:00:00 2001 From: Basit Chonka Date: Thu, 1 Aug 2024 17:06:12 +0200 Subject: [PATCH 1/4] prompt for password when redirecting output --- packages/cli-repl/src/cli-repl.ts | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/packages/cli-repl/src/cli-repl.ts b/packages/cli-repl/src/cli-repl.ts index d390ea107..3cadccc4b 100644 --- a/packages/cli-repl/src/cli-repl.ts +++ b/packages/cli-repl/src/cli-repl.ts @@ -1003,22 +1003,23 @@ export class CliRepl implements MongoshIOProvider { /** * Require the user to enter a password. - * - * @param {string} driverUrl - The driver URI. - * @param {DevtoolsConnectOptions} driverOptions - The driver options. */ async requirePassword(): Promise { + const printPrompt = process.stdout.isTTY + ? this.output.write.bind(this.output) + : process.stderr.write.bind(process.stderr); + const passwordPromise = askpassword({ input: this.input, - output: this.output, + output: process.stdout.isTTY ? this.output : process.stderr, replacementCharacter: '*', }); - this.output.write('Enter password: '); + printPrompt('Enter password: '); try { try { return (await passwordPromise).toString(); } finally { - this.output.write('\n'); + printPrompt('\n'); } } catch (error: any) { await this._fatalError(error); From 1b38d442627bbad5ce3818a5da4ca9c494a835eb Mon Sep 17 00:00:00 2001 From: Basit Chonka Date: Mon, 5 Aug 2024 10:11:53 +0200 Subject: [PATCH 2/4] inject promptOutput --- packages/cli-repl/src/cli-repl.ts | 18 +++++++++++------- packages/cli-repl/src/run.ts | 1 + 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/packages/cli-repl/src/cli-repl.ts b/packages/cli-repl/src/cli-repl.ts index 3cadccc4b..cd75f9d55 100644 --- a/packages/cli-repl/src/cli-repl.ts +++ b/packages/cli-repl/src/cli-repl.ts @@ -78,6 +78,12 @@ export type CliReplOptions = { input: Readable; /** The stream to write shell output to. */ output: Writable; + /** + * The stream to write prompt output to when requesting data from user, like password. + * Helpful when user wants to redirect the output to a file or null device. + * If not provided, the `output` stream will be used. + */ + promptOutput?: Writable; /** The set of home directory paths used by this shell instance. */ shellHomePaths: ShellHomePaths; /** The ordered list of paths in which to look for a global configuration file. */ @@ -112,6 +118,7 @@ export class CliRepl implements MongoshIOProvider { logWriter?: MongoLogWriter; input: Readable; output: Writable; + promptOutput: Writable; analyticsOptions?: AnalyticsOptions; segmentAnalytics?: SegmentAnalytics; toggleableAnalytics: ToggleableAnalytics = new ToggleableAnalytics(); @@ -132,6 +139,7 @@ export class CliRepl implements MongoshIOProvider { this.cliOptions = options.shellCliOptions; this.input = options.input; this.output = options.output; + this.promptOutput = options.promptOutput ?? options.output; this.analyticsOptions = options.analyticsOptions; this.onExit = options.onExit; @@ -1005,21 +1013,17 @@ export class CliRepl implements MongoshIOProvider { * Require the user to enter a password. */ async requirePassword(): Promise { - const printPrompt = process.stdout.isTTY - ? this.output.write.bind(this.output) - : process.stderr.write.bind(process.stderr); - const passwordPromise = askpassword({ input: this.input, - output: process.stdout.isTTY ? this.output : process.stderr, + output: this.promptOutput, replacementCharacter: '*', }); - printPrompt('Enter password: '); + this.promptOutput.write('Enter password: '); try { try { return (await passwordPromise).toString(); } finally { - printPrompt('\n'); + this.promptOutput.write('\n'); } } catch (error: any) { await this._fatalError(error); diff --git a/packages/cli-repl/src/run.ts b/packages/cli-repl/src/run.ts index 9ef5fe304..ad85deadb 100644 --- a/packages/cli-repl/src/run.ts +++ b/packages/cli-repl/src/run.ts @@ -217,6 +217,7 @@ async function main() { getCryptLibraryPaths, input: process.stdin, output: process.stdout, + promptOutput: process.stdout.isTTY ? process.stdout : process.stderr, // Node.js 20.0.0 made p.exit(undefined) behave as p.exit(0) rather than p.exit() onExit: (code?: number | undefined) => code === undefined ? process.exit() : process.exit(code), From 2b676ee46805ca67cb86dbc970ca67070954c740 Mon Sep 17 00:00:00 2001 From: Basit Chonka Date: Mon, 12 Aug 2024 14:50:06 +0200 Subject: [PATCH 3/4] add tests --- packages/cli-repl/src/run.spec.ts | 48 +++++++++++++++++++++++++++++++ packages/cli-repl/src/run.ts | 5 +++- 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/packages/cli-repl/src/run.spec.ts b/packages/cli-repl/src/run.spec.ts index 5f4497826..b0c4e767e 100644 --- a/packages/cli-repl/src/run.spec.ts +++ b/packages/cli-repl/src/run.spec.ts @@ -97,4 +97,52 @@ describe('CLI entry point', function () { 'MongoshInvalidInputError: [COMMON-10001] Invalid URI: /' ); }); + + context('prompts for password', function () { + it('requests password when user is not redirecting output', async function () { + const args = [...pathToRun, 'mongodb://amy@localhost:27017']; + const proc = childProcess.spawn(process.execPath, args, { + stdio: ['pipe'], + env: { ...process.env, TEST_USE_STDOUT_FOR_PASSWORD: '1' }, + }); + let stdout = ''; + let promptedForPassword = false; + proc.stdout?.setEncoding('utf8').on('data', (chunk) => { + stdout += chunk; + if (stdout.includes('Enter password')) { + promptedForPassword = true; + proc.stdin?.write('\n'); + } + }); + + const [code] = await once(proc, 'exit'); + expect(code).to.equal(1); + expect(promptedForPassword).to.be.true; + }); + + it('requests password when user is redirecting output', async function () { + const args = [ + ...pathToRun, + 'mongodb://amy@localhost:27017', + '> /dev/null', + ]; + const proc = childProcess.spawn(process.execPath, args, { + stdio: 'pipe', + env: { ...process.env }, + }); + let stderr = ''; + let promptedForPassword = false; + proc.stderr?.setEncoding('utf8').on('data', (chunk) => { + stderr += chunk; + if (stderr.includes('Enter password')) { + promptedForPassword = true; + proc.stdin?.write('\n'); + } + }); + + const [code] = await once(proc, 'exit'); + expect(code).to.equal(1); + expect(promptedForPassword).to.be.true; + }); + }); }); diff --git a/packages/cli-repl/src/run.ts b/packages/cli-repl/src/run.ts index ad85deadb..f97e598a7 100644 --- a/packages/cli-repl/src/run.ts +++ b/packages/cli-repl/src/run.ts @@ -217,7 +217,10 @@ async function main() { getCryptLibraryPaths, input: process.stdin, output: process.stdout, - promptOutput: process.stdout.isTTY ? process.stdout : process.stderr, + promptOutput: + process.env.TEST_USE_STDOUT_FOR_PASSWORD || process.stdout.isTTY + ? process.stdout + : process.stderr, // Node.js 20.0.0 made p.exit(undefined) behave as p.exit(0) rather than p.exit() onExit: (code?: number | undefined) => code === undefined ? process.exit() : process.exit(code), From 014998277a027589b7d1210b62fd584e1e37d77f Mon Sep 17 00:00:00 2001 From: Basit Chonka Date: Mon, 12 Aug 2024 16:11:49 +0200 Subject: [PATCH 4/4] feedback --- packages/cli-repl/src/run.spec.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/packages/cli-repl/src/run.spec.ts b/packages/cli-repl/src/run.spec.ts index b0c4e767e..c47a49e5a 100644 --- a/packages/cli-repl/src/run.spec.ts +++ b/packages/cli-repl/src/run.spec.ts @@ -121,13 +121,9 @@ describe('CLI entry point', function () { }); it('requests password when user is redirecting output', async function () { - const args = [ - ...pathToRun, - 'mongodb://amy@localhost:27017', - '> /dev/null', - ]; + const args = [...pathToRun, 'mongodb://amy@localhost:27017']; const proc = childProcess.spawn(process.execPath, args, { - stdio: 'pipe', + stdio: ['pipe', 'ignore', 'pipe'], env: { ...process.env }, }); let stderr = '';