Skip to content

Commit 02cf2ef

Browse files
committed
Couple of fixes:
- Previous sudo check blocked the main thread - Move title of sudo inside of the box - Make sure not to cache password for regular plugin requests
1 parent 0358b6d commit 02cf2ef

5 files changed

Lines changed: 49 additions & 20 deletions

File tree

src/common/base-command.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,8 @@ export abstract class BaseCommand extends Command {
5252
this.reporter.notifySudoPasswordPreSupplied();
5353
}
5454

55-
this.reporter.onSudoPasswordSubmitted((password: string) => {
56-
const isValid = SudoUtils.validate(password);
55+
this.reporter.onSudoPasswordSubmitted(async (password: string) => {
56+
const isValid = await SudoUtils.validate(password);
5757
if (isValid) {
5858
cachedSudoPassword = password;
5959
}

src/ui/components/default-component.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ export function DefaultComponent(props: {
8585
renderStatus === RenderStatus.SUDO_PROMPT && (
8686
<SudoPasswordInput
8787
key={(renderData as { attemptCount: number }).attemptCount}
88+
title={(renderData as { title?: string }).title}
8889
hasError={(renderData as { attemptCount: number }).attemptCount > 0}
8990
cancellable={(renderData as { cancellable: boolean }).cancellable}
9091
onSubmit={(password) => emitter.emit(RenderEvent.SUDO_PROMPT_RESULT, password)}

src/ui/components/widgets/SudoPasswordInput.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,13 @@ import React, { useState } from 'react';
44
import Spinner from '../progress/spinner.js';
55

66
export function SudoPasswordInput(props: {
7+
title?: string;
78
hasError: boolean;
89
cancellable: boolean;
910
onSubmit: (password: string) => void;
1011
onCancel: () => void;
1112
}) {
12-
const { hasError, cancellable, onSubmit, onCancel } = props;
13+
const { title, hasError, cancellable, onSubmit, onCancel } = props;
1314
const [value, setValue] = useState('');
1415
const [isChecking, setIsChecking] = useState(false);
1516

@@ -52,6 +53,7 @@ export function SudoPasswordInput(props: {
5253
borderColor={borderColor}
5354
marginTop={1}
5455
>
56+
{title && <Text bold>{title}</Text>}
5557
{isChecking ? (
5658
<Spinner label="Checking password..." />
5759
) : (

src/ui/reporters/default-reporter.tsx

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ export class DefaultReporter implements Reporter {
4747
private renderEmitter = new EventEmitter();
4848
private progressState: ProgressState | null = null
4949
private verbosityToggleCallback: (() => void) | null = null;
50-
private sudoPasswordSubmittedCallback: ((password: string) => boolean) | null = null;
50+
private sudoPasswordSubmittedCallback: ((password: string) => Promise<boolean>) | null = null;
5151
silent = false;
5252

5353
constructor() {
@@ -72,7 +72,7 @@ export class DefaultReporter implements Reporter {
7272
this.verbosityToggleCallback = callback;
7373
}
7474

75-
onSudoPasswordSubmitted(callback: (password: string) => boolean): void {
75+
onSudoPasswordSubmitted(callback: (password: string) => Promise<boolean>): void {
7676
this.sudoPasswordSubmittedCallback = callback;
7777
}
7878

@@ -199,13 +199,14 @@ export class DefaultReporter implements Reporter {
199199
}
200200

201201
async promptSudo(pluginName: string, data: CommandRequestData, secureMode: boolean): Promise<string | undefined> {
202-
ctx.log(chalk.blue(`Plugin: "${pluginName}" requires root access to run command: "sudo ${data.command}"`));
202+
ctx.log(`Plugin: "${pluginName}" requires root access to run command: "sudo ${data.command}"`);
203+
const title = `Plugin: "${pluginName}" requires root access to run command: "sudo ${data.command}"`;
203204

204205
let password;
205206

206207
// Password is only needed outside of sudo timeout. Pass password in as undefined if not needed.
207-
if (secureMode || !SudoUtils.validate()) {
208-
password = await this.getUserPassword();
208+
if (secureMode || !(await SudoUtils.validate())) {
209+
password = await this.getUserPassword(title);
209210
}
210211

211212
return password;
@@ -332,7 +333,7 @@ export class DefaultReporter implements Reporter {
332333
ctx.log('Sudo password attempt');
333334
}
334335

335-
const isValid = this.sudoPasswordSubmittedCallback?.(result as string) ?? false;
336+
const isValid = await (this.sudoPasswordSubmittedCallback?.(result as string) ?? Promise.resolve(false));
336337
if (isValid) {
337338
ctx.log('Sudo password successful!');
338339

@@ -349,17 +350,21 @@ export class DefaultReporter implements Reporter {
349350
await this.displayProgress();
350351
}
351352

352-
private async getUserPassword(): Promise<string> {
353+
private async getUserPassword(title?: string): Promise<string> {
353354
let attemptCount = 0;
354355

355356
while (attemptCount < 3) {
356357
const passwordAttempt = await this.updateStateAndAwaitEvent<string>(
357-
() => this.updateRenderState(RenderStatus.SUDO_PROMPT, { attemptCount, cancellable: false }),
358+
() => this.updateRenderState(RenderStatus.SUDO_PROMPT, { attemptCount, cancellable: false, title }),
358359
RenderEvent.SUDO_PROMPT_RESULT,
359360
);
360361

361362
// Validates that the password works
362-
if (SudoUtils.validate(passwordAttempt)) {
363+
if (await SudoUtils.validate(passwordAttempt)) {
364+
// Drop the sudo session so it is not cached for future prompts.
365+
// The inline sudo path (handleInlineSudoPassword) caches intentionally;
366+
// this path (promptSudo) must not.
367+
await SudoUtils.invalidate();
363368
await this.displayProgress();
364369
return passwordAttempt;
365370
}

src/utils/sudo.ts

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,43 @@
1-
import { execSync } from 'node:child_process';
1+
import { execFile, spawn } from 'node:child_process';
2+
import util from 'node:util';
23

34
import { OsUtils } from './os-utils.js';
45
import { ShellUtils } from './shell.js';
56

7+
const execFileAsync = util.promisify(execFile);
8+
9+
function spawnWithInput(command: string, args: string[], input?: string): Promise<boolean> {
10+
return new Promise((resolve) => {
11+
const child = spawn(command, args, { stdio: ['pipe', 'ignore', 'ignore'] });
12+
child.on('error', () => resolve(false));
13+
child.on('close', (code) => resolve(code === 0));
14+
if (input) {
15+
child.stdin.end(input + '\n');
16+
} else {
17+
child.stdin.end();
18+
}
19+
});
20+
}
21+
622
export const SudoUtils = {
7-
validate(password?: string): boolean {
23+
async validate(password?: string): Promise<boolean> {
824
try {
925
if (OsUtils.isMacOS()) {
1026
// Sudo with -SNv will not prompt if within sudo cache timeout. Mac OS uses -SNv instead of -Snv
11-
execSync(`sudo -SNv ${password ? `<<< ${password}` : ''}`, { stdio: 'ignore' })
12-
return true;
27+
return await spawnWithInput('sudo', ['-SNv'], password);
1328
}
1429

15-
// Sudo with -Snv will not prompt if within sudo cache timeout
16-
execSync(`sudo -Skv ${password ? `<<< '${password}'` : ''}`, { stdio: 'ignore', shell: ShellUtils.getDefaultShell() })
17-
return true;
30+
// Sudo with -Skv will not prompt if within sudo cache timeout
31+
const shell = ShellUtils.getDefaultShell();
32+
return await spawnWithInput(shell, ['-c', 'sudo -Skv'], password);
1833
} catch {
1934
return false;
2035
}
21-
}
36+
},
37+
38+
async invalidate(): Promise<void> {
39+
try {
40+
await execFileAsync('sudo', ['-k']);
41+
} catch { /* sudo -k never fails meaningfully */ }
42+
},
2243
};

0 commit comments

Comments
 (0)