From c25d0b8e61eca36024cbd65945ec955e73f4f302 Mon Sep 17 00:00:00 2001 From: Chiara Mooney <34109996+chiaramooney@users.noreply.github.com> Date: Wed, 15 Jul 2026 13:42:18 -0700 Subject: [PATCH 1/4] Route cert install through an elevated window instead of failing (#34) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Installing a development certificate trusts it in the machine store, which requires administrator rights. VS Code isn't elevated by default and cannot create an elevated integrated terminal, so `cert generate --install` and `cert install` printed a red "Access denied" failure even though the cert was generated — a scary, misleading first-run experience. - Detect whether VS Code is elevated (WindowsPrincipal check). When elevated, run the install in the normal terminal as before. - When not elevated, launch the command in a separate UAC-elevated PowerShell window via `Start-Process -Verb RunAs`, and show an info message so the user knows to approve the prompt. The elevated shell is pointed back at the working directory with `Set-Location` (RunAs otherwise starts in System32, which would break `cert generate` publisher inference and misplace devcert.pfx). - Default the Generate Certificate quick pick to "Generate only" so users opt into the admin step instead of hitting it by surprise. - Extract `buildElevatedTerminalCommand` into winapp-cli-utils with unit tests covering escaping, the RunAs verb, and the working-directory fix. - Update README command table and troubleshooting notes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5ce7415c-8966-43ec-ad27-59a68e7ed74b --- README.md | 8 ++-- src/extension.ts | 84 +++++++++++++++++++++++++++++++---- src/test/shell-escape.test.ts | 44 +++++++++++++++++- src/winapp-cli-utils.ts | 33 ++++++++++++++ 4 files changed, 155 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index 7dbdeab..9345260 100644 --- a/README.md +++ b/README.md @@ -32,8 +32,8 @@ All commands are accessible from the Command Palette (`Ctrl+Shift+P`). Type **Wi | **WinApp: Generate Manifest** | Generate an `AppxManifest.xml` from a template (packaged or sparse). | | **WinApp: Add Manifest Execution Alias** | Add an execution alias to the manifest so the packaged app can be launched from the command line. | | **WinApp: Update Manifest Assets** | Auto-generate all required app icon assets from a single source image (PNG, JPG, GIF, or BMP). | -| **WinApp: Generate Certificate** | Create a development certificate for signing, with an option to install it immediately. | -| **WinApp: Install Certificate** | Install an existing `.pfx` or `.cer` certificate. (requires Admin elevation) | +| **WinApp: Generate Certificate** | Create a development certificate for signing, with an option to also install (trust) it. Installing prompts for admin via a UAC window when VS Code isn't elevated. | +| **WinApp: Install Certificate** | Install (trust) an existing `.pfx` or `.cer` certificate in the machine store. Prompts for admin via a UAC window when VS Code isn't elevated. | | **WinApp: Certificate Info** | Display certificate details (subject, thumbprint, expiry) to verify a certificate matches your manifest. | | **WinApp: Sign Package** | Sign an MSIX package or executable with a certificate. | | **WinApp: Run SDK Tool** | Run Windows SDK tools (`makeappx`, `signtool`, `mt`, `makepri`) with custom arguments. | @@ -245,8 +245,8 @@ For debugging, install the debugger extension that matches your app's language ( | **"No folders containing .exe files found in the workspace..."** or **"No build output folder selected..."** when pressing F5 | The project hasn't been built yet, or the build output is in an unexpected location. | Build your project first (e.g., `dotnet build`), or set `inputFolder` in `launch.json` to point to the folder containing your `.exe`. | | **Debugger doesn't attach** | The required debugger extension isn't installed. | Install the matching extension for your language — see [Supported debuggers](#integrated-debugging). | | **App launches but changes aren't visible** | The `winapp` debug type does not build the project automatically. | Rebuild your project before pressing F5, or add a `preLaunchTask` to automate it (see the tip in [Integrated Debugging](#integrated-debugging)). | -| **Certificate trust error when running** | The development certificate isn't installed or has expired. | Run **WinApp: Generate Certificate** and choose to install it, or run **WinApp: Install Certificate** with your existing `.pfx` file. (requires Admin elevation) | -| **"Access denied" or permission errors** | Some operations (certificate install, package registration) require elevation. | Run VS Code as Administrator, or use an elevated terminal for the failing command. | +| **Certificate trust error when running** | The development certificate isn't installed or has expired. | Run **WinApp: Generate Certificate** and choose to also install it, or run **WinApp: Install Certificate** with your existing `.pfx` file. Both prompt for admin (UAC) when VS Code isn't elevated. | +| **"Access denied" or permission errors** | Some operations (package registration) require elevation. Certificate install now prompts for admin automatically. | Approve the UAC prompt when it appears, or run VS Code as Administrator. | ## Feedback and Support diff --git a/src/extension.ts b/src/extension.ts index 2bdba26..d4f5082 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -1,7 +1,7 @@ import * as vscode from 'vscode'; import * as path from 'path'; -import { spawn } from 'child_process'; -import { getWinappCliPath, WINAPP_CLI_CALLER_VALUE, escapePowerShellArg } from './winapp-cli-utils'; +import { spawn, execFile } from 'child_process'; +import { getWinappCliPath, WINAPP_CLI_CALLER_VALUE, escapePowerShellArg, buildElevatedTerminalCommand } from './winapp-cli-utils'; import { detectProjects } from './project-detection'; import { resolveProjectDirectory as resolveProjectDirectoryCore } from './project-resolver'; import { glob } from 'glob'; @@ -69,6 +69,63 @@ async function runWinappCommand(extensionPath: string, command: string, cwd: str return ''; } +/** + * Report whether the current VS Code process is running elevated (as + * administrator). + * + * VS Code cannot launch an elevated integrated terminal, so admin-only winapp + * commands must be routed either through the normal terminal (when already + * elevated) or through a separate UAC-elevated window (when not). We probe the + * process token with PowerShell's WindowsPrincipal check. On any failure we + * return `false`, which is the safe default: the command is then launched via + * an elevated window (UAC), avoiding a silent "Access denied" failure. + */ +async function isProcessElevated(): Promise { + return new Promise((resolve) => { + execFile( + 'powershell.exe', + [ + '-NoProfile', + '-NonInteractive', + '-Command', + '[bool]([Security.Principal.WindowsPrincipal][Security.Principal.WindowsIdentity]::GetCurrent()).IsInRole([Security.Principal.WindowsBuiltInRole]::Administrator)' + ], + { timeout: 10000, windowsHide: true }, + (error, stdout) => { + resolve(!error && stdout.trim().toLowerCase() === 'true'); + } + ); + }); +} + +/** + * Run a winapp command that requires administrator rights. + * + * If VS Code is already elevated, the command runs in the normal integrated + * terminal. Otherwise it is launched in a separate UAC-elevated PowerShell + * window (VS Code cannot elevate the integrated terminal), and an information + * message explains that a Windows admin prompt will appear. + */ +async function runWinappCommandElevated(extensionPath: string, command: string, cwd: string): Promise { + if (await isProcessElevated()) { + await runWinappCommand(extensionPath, command, cwd); + return; + } + + const cliPath = getWinappCliPath(extensionPath); + const terminal = vscode.window.createTerminal({ + name: 'WinApp CLI (Admin)', + cwd: cwd, + shellPath: 'powershell.exe', + env: { WINAPP_CLI_CALLER: WINAPP_CLI_CALLER_VALUE } + }); + terminal.show(); + terminal.sendText(buildElevatedTerminalCommand(cliPath, command, cwd)); + vscode.window.showInformationMessage( + 'Installing the certificate requires administrator rights. Approve the Windows User Account Control (UAC) prompt in the elevated window that just opened.' + ); +} + /** * Resolves the project directory for commands that need a winapp project context. * Priority: 1) winapp.appDirectories setting, 2) project at workspace root, 3) scan workspace. @@ -656,16 +713,23 @@ export function activate(context: vscode.ExtensionContext) { } const install = await vscode.window.showQuickPick( - ['Yes', 'No'], - { placeHolder: 'Install certificate after generation?' } + ['Generate only', 'Generate and install (requires admin)'], + { placeHolder: 'Generate a development certificate — install it in the machine store too?' } ); - let command = 'cert generate'; - if (install === 'Yes') { - command += ' --install'; + if (!install) { + return; } - await runWinappCommand(extensionPath, command, projectDir); + // Installing trusts the certificate in the machine store, which needs + // administrator rights. When VS Code isn't elevated we can't install + // from the integrated terminal, so run the whole generate+install in a + // separate UAC-elevated window instead of failing with "Access denied". + if (install === 'Generate and install (requires admin)') { + await runWinappCommandElevated(extensionPath, 'cert generate --install', projectDir); + } else { + await runWinappCommand(extensionPath, 'cert generate', projectDir); + } }) ); @@ -686,7 +750,9 @@ export function activate(context: vscode.ExtensionContext) { return; } - await runWinappCommand(extensionPath, `cert install ${escapePowerShellArg(certPath)}`, workspacePath); + // Trusting a certificate in the machine store requires admin; route + // through an elevated window when VS Code isn't already elevated. + await runWinappCommandElevated(extensionPath, `cert install ${escapePowerShellArg(certPath)}`, workspacePath); }) ); diff --git a/src/test/shell-escape.test.ts b/src/test/shell-escape.test.ts index dd1685f..7be159c 100644 --- a/src/test/shell-escape.test.ts +++ b/src/test/shell-escape.test.ts @@ -1,6 +1,6 @@ import { describe, it } from 'node:test'; import assert from 'node:assert/strict'; -import { escapePowerShellArg } from '../winapp-cli-utils'; +import { escapePowerShellArg, buildElevatedTerminalCommand } from '../winapp-cli-utils'; describe('escapePowerShellArg', () => { it('wraps an ordinary path in single quotes', () => { @@ -36,3 +36,45 @@ describe('escapePowerShellArg', () => { assert.equal(escapePowerShellArg(''), "''"); }); }); + +describe('buildElevatedTerminalCommand', () => { + it('launches an elevated PowerShell window that runs the winapp CLI in the working directory', () => { + const result = buildElevatedTerminalCommand( + 'C:\\ext\\bin\\winapp.exe', + "cert install 'C:\\proj\\devcert.pfx'", + 'C:\\proj' + ); + assert.equal( + result, + "Start-Process -FilePath 'powershell.exe' -Verb RunAs -ArgumentList '-NoExit', '-Command', " + + "'Set-Location -LiteralPath ''C:\\proj''; & ''C:\\ext\\bin\\winapp.exe'' cert install ''C:\\proj\\devcert.pfx'''" + ); + }); + + it('requests elevation via the RunAs verb and keeps the window open', () => { + const result = buildElevatedTerminalCommand('winapp', 'cert generate --install', 'C:\\proj'); + assert.match(result, /-Verb RunAs/); + assert.match(result, /-NoExit/); + }); + + it('sets the elevated working directory so RunAs does not default to System32', () => { + const result = buildElevatedTerminalCommand('winapp', 'cert generate', 'C:\\my proj'); + // Doubled quotes because Set-Location sits inside the outer -Command literal. + assert.match(result, /Set-Location -LiteralPath ''C:\\my proj''/); + }); + + it('doubles single quotes in the CLI path so it cannot break out of the literal', () => { + const result = buildElevatedTerminalCommand("C:\\o'brien\\winapp.exe", 'cert generate', 'C:\\proj'); + // The path's quote is doubled once for the inner call and again for the + // outer -Command literal, leaving every quote in the line balanced. + assert.equal((result.match(/'/g) || []).length % 2, 0); + assert.ok(result.includes("o''''brien")); + }); + + it('keeps a quote-injection attempt in the args inert', () => { + const attack = escapePowerShellArg("'; Remove-Item C:\\ #"); + const result = buildElevatedTerminalCommand('winapp', `cert install ${attack}`, 'C:\\proj'); + // Every quote is paired: the injection stays inside a single literal. + assert.equal((result.match(/'/g) || []).length % 2, 0); + }); +}); diff --git a/src/winapp-cli-utils.ts b/src/winapp-cli-utils.ts index d550b45..063c836 100644 --- a/src/winapp-cli-utils.ts +++ b/src/winapp-cli-utils.ts @@ -40,3 +40,36 @@ export function getWinappCliPath(extensionPath: string): string { export function escapePowerShellArg(value: string): string { return `'${value.replace(/'/g, "''")}'`; } + +/** + * Build a PowerShell command that runs the winapp CLI elevated in a *separate* + * console via `Start-Process -Verb RunAs`. + * + * VS Code cannot create an elevated integrated terminal (there is no extension + * API for it, by design). Commands that need administrator rights — notably + * `cert install`, which trusts a certificate in the machine store — are + * therefore launched in a new UAC-elevated PowerShell window. `-NoExit` keeps + * that window open so the user can read the result and any errors. + * + * The elevated shell is pointed at `workingDirectory` with `Set-Location` + * before invoking the CLI. A RunAs-elevated process otherwise starts in + * `System32`, which would break commands that rely on the working directory + * (e.g. `cert generate` infers the publisher from the manifest in the current + * directory and writes `devcert.pfx` there). `Set-Location` is used instead of + * `Start-Process -WorkingDirectory` because the latter is ignored with `-Verb` + * on Windows PowerShell 5.1. + * + * The working directory, CLI path, and the composed command are all quoted with + * {@link escapePowerShellArg}, so values containing single quotes stay balanced + * and cannot break out of the literal. + * + * @param cliPath Path to the bundled winapp executable. + * @param cliArgs The winapp arguments, already PowerShell-escaped where needed + * (e.g. `cert install 'C:\path\devcert.pfx'`). + * @param workingDirectory Directory to run the elevated command in. + * @returns A PowerShell command line to send to a (non-elevated) terminal. + */ +export function buildElevatedTerminalCommand(cliPath: string, cliArgs: string, workingDirectory: string): string { + const innerCommand = `Set-Location -LiteralPath ${escapePowerShellArg(workingDirectory)}; & ${escapePowerShellArg(cliPath)} ${cliArgs}`.trim(); + return `Start-Process -FilePath 'powershell.exe' -Verb RunAs -ArgumentList '-NoExit', '-Command', ${escapePowerShellArg(innerCommand)}`; +} From 76836b1f0a445acaeab22a3cab5f1467e95e5f6d Mon Sep 17 00:00:00 2001 From: Chiara Mooney <34109996+chiaramooney@users.noreply.github.com> Date: Wed, 15 Jul 2026 14:03:55 -0700 Subject: [PATCH 2/4] Address PR review: harden elevated launch paths and cover routing (#34) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5ce7415c-8966-43ec-ad27-59a68e7ed74b --- src/extension.ts | 34 ++++++++++-- src/test/shell-escape.test.ts | 101 +++++++++++++++++++++++++++++++--- src/winapp-cli-utils.ts | 37 ++++++++++++- 3 files changed, 158 insertions(+), 14 deletions(-) diff --git a/src/extension.ts b/src/extension.ts index d4f5082..4051ebd 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -1,7 +1,15 @@ import * as vscode from 'vscode'; import * as path from 'path'; +import * as fs from 'fs'; import { spawn, execFile } from 'child_process'; -import { getWinappCliPath, WINAPP_CLI_CALLER_VALUE, escapePowerShellArg, buildElevatedTerminalCommand } from './winapp-cli-utils'; +import { + getWinappCliPath, + WINAPP_CLI_CALLER_VALUE, + escapePowerShellArg, + getWindowsPowerShellPath, + isUsableElevatedCliPath, + decideElevatedWinappCommand +} from './winapp-cli-utils'; import { detectProjects } from './project-detection'; import { resolveProjectDirectory as resolveProjectDirectoryCore } from './project-resolver'; import { glob } from 'glob'; @@ -107,12 +115,30 @@ async function isProcessElevated(): Promise { * message explains that a Windows admin prompt will appear. */ async function runWinappCommandElevated(extensionPath: string, command: string, cwd: string): Promise { - if (await isProcessElevated()) { + const isElevated = await isProcessElevated(); + const cliPath = getWinappCliPath(extensionPath); + const launcherPath = getWindowsPowerShellPath(process.env.SystemRoot); + const decision = decideElevatedWinappCommand( + isElevated, + isUsableElevatedCliPath(cliPath, fs.existsSync(cliPath)), + cliPath, + command, + cwd, + launcherPath + ); + + if (decision.kind === 'run-normally') { await runWinappCommand(extensionPath, command, cwd); return; } - const cliPath = getWinappCliPath(extensionPath); + if (decision.kind === 'error-cli-missing') { + vscode.window.showErrorMessage( + 'The bundled WinApp CLI executable could not be found, so the administrator command was not started. Rebuild or reinstall the extension, then try again.' + ); + return; + } + const terminal = vscode.window.createTerminal({ name: 'WinApp CLI (Admin)', cwd: cwd, @@ -120,7 +146,7 @@ async function runWinappCommandElevated(extensionPath: string, command: string, env: { WINAPP_CLI_CALLER: WINAPP_CLI_CALLER_VALUE } }); terminal.show(); - terminal.sendText(buildElevatedTerminalCommand(cliPath, command, cwd)); + terminal.sendText(decision.command); vscode.window.showInformationMessage( 'Installing the certificate requires administrator rights. Approve the Windows User Account Control (UAC) prompt in the elevated window that just opened.' ); diff --git a/src/test/shell-escape.test.ts b/src/test/shell-escape.test.ts index 7be159c..19a8fb8 100644 --- a/src/test/shell-escape.test.ts +++ b/src/test/shell-escape.test.ts @@ -1,6 +1,14 @@ import { describe, it } from 'node:test'; import assert from 'node:assert/strict'; -import { escapePowerShellArg, buildElevatedTerminalCommand } from '../winapp-cli-utils'; +import { + escapePowerShellArg, + buildElevatedTerminalCommand, + getWindowsPowerShellPath, + isUsableElevatedCliPath, + decideElevatedWinappCommand +} from '../winapp-cli-utils'; + +const launcherPath = 'C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe'; describe('escapePowerShellArg', () => { it('wraps an ordinary path in single quotes', () => { @@ -8,7 +16,7 @@ describe('escapePowerShellArg', () => { }); it('does not expand subexpression syntax', () => { - // A double-quoted PowerShell string would evaluate $(...) — a single-quoted + // A double-quoted PowerShell string would evaluate $(...) - a single-quoted // literal must keep it verbatim so the path cannot run commands. const malicious = 'C:\\$(Remove-Item -Recurse C:\\)'; assert.equal(escapePowerShellArg(malicious), `'${malicious}'`); @@ -42,29 +50,42 @@ describe('buildElevatedTerminalCommand', () => { const result = buildElevatedTerminalCommand( 'C:\\ext\\bin\\winapp.exe', "cert install 'C:\\proj\\devcert.pfx'", - 'C:\\proj' + 'C:\\proj', + launcherPath ); assert.equal( result, - "Start-Process -FilePath 'powershell.exe' -Verb RunAs -ArgumentList '-NoExit', '-Command', " + + "Start-Process -FilePath 'C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe' -Verb RunAs -ArgumentList '-NoExit', '-Command', " + "'Set-Location -LiteralPath ''C:\\proj''; & ''C:\\ext\\bin\\winapp.exe'' cert install ''C:\\proj\\devcert.pfx'''" ); }); it('requests elevation via the RunAs verb and keeps the window open', () => { - const result = buildElevatedTerminalCommand('winapp', 'cert generate --install', 'C:\\proj'); + const result = buildElevatedTerminalCommand('winapp', 'cert generate --install', 'C:\\proj', launcherPath); assert.match(result, /-Verb RunAs/); assert.match(result, /-NoExit/); }); it('sets the elevated working directory so RunAs does not default to System32', () => { - const result = buildElevatedTerminalCommand('winapp', 'cert generate', 'C:\\my proj'); + const result = buildElevatedTerminalCommand('winapp', 'cert generate', 'C:\\my proj', launcherPath); // Doubled quotes because Set-Location sits inside the outer -Command literal. assert.match(result, /Set-Location -LiteralPath ''C:\\my proj''/); }); + it('uses the supplied absolute Windows PowerShell launcher path', () => { + const customLauncher = 'D:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe'; + const result = buildElevatedTerminalCommand('C:\\ext\\bin\\winapp.exe', 'cert generate', 'C:\\proj', customLauncher); + assert.match(result, /^Start-Process -FilePath 'D:\\Windows\\System32\\WindowsPowerShell\\v1\.0\\powershell\.exe'/); + }); + + it('doubles single quotes in the working directory so it cannot break out of Set-Location', () => { + const result = buildElevatedTerminalCommand('C:\\ext\\bin\\winapp.exe', 'cert generate', "C:\\o'brien\\proj", launcherPath); + assert.match(result, /Set-Location -LiteralPath ''C:\\o''''brien\\proj''/); + assert.equal((result.match(/'/g) || []).length % 2, 0); + }); + it('doubles single quotes in the CLI path so it cannot break out of the literal', () => { - const result = buildElevatedTerminalCommand("C:\\o'brien\\winapp.exe", 'cert generate', 'C:\\proj'); + const result = buildElevatedTerminalCommand("C:\\o'brien\\winapp.exe", 'cert generate', 'C:\\proj', launcherPath); // The path's quote is doubled once for the inner call and again for the // outer -Command literal, leaving every quote in the line balanced. assert.equal((result.match(/'/g) || []).length % 2, 0); @@ -73,8 +94,72 @@ describe('buildElevatedTerminalCommand', () => { it('keeps a quote-injection attempt in the args inert', () => { const attack = escapePowerShellArg("'; Remove-Item C:\\ #"); - const result = buildElevatedTerminalCommand('winapp', `cert install ${attack}`, 'C:\\proj'); + const result = buildElevatedTerminalCommand('winapp', `cert install ${attack}`, 'C:\\proj', launcherPath); // Every quote is paired: the injection stays inside a single literal. assert.equal((result.match(/'/g) || []).length % 2, 0); }); }); + +describe('getWindowsPowerShellPath', () => { + it('builds the launcher path from SystemRoot', () => { + assert.equal(getWindowsPowerShellPath('D:\\Windows'), 'D:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe'); + }); + + it('falls back to C:\\Windows when SystemRoot is unavailable', () => { + assert.equal(getWindowsPowerShellPath(), launcherPath); + }); +}); + +describe('isUsableElevatedCliPath', () => { + it('accepts an absolute existing winapp.exe path', () => { + assert.equal(isUsableElevatedCliPath('C:\\ext\\bin\\winapp.exe', true), true); + }); + + it('rejects the search-path fallback even if a command by that name exists', () => { + assert.equal(isUsableElevatedCliPath('winapp', true), false); + }); + + it('rejects an absolute winapp.exe path when it does not exist', () => { + assert.equal(isUsableElevatedCliPath('C:\\ext\\bin\\winapp.exe', false), false); + }); + + it('rejects an absolute path to a different executable', () => { + assert.equal(isUsableElevatedCliPath('C:\\ext\\bin\\powershell.exe', true), false); + }); +}); + +describe('decideElevatedWinappCommand', () => { + it('routes elevated processes to the normal terminal without requiring an elevated-safe CLI path', () => { + assert.deepEqual( + decideElevatedWinappCommand(true, false, 'winapp', 'cert generate --install', 'C:\\proj', launcherPath), + { kind: 'run-normally' } + ); + }); + + it('routes non-elevated processes with a usable CLI path to an elevated launcher command', () => { + const decision = decideElevatedWinappCommand( + false, + true, + 'C:\\ext\\bin\\winapp.exe', + 'cert generate --install', + 'C:\\proj', + launcherPath + ); + + assert.equal(decision.kind, 'run-elevated'); + if (decision.kind === 'run-elevated') { + assert.equal( + decision.command, + buildElevatedTerminalCommand('C:\\ext\\bin\\winapp.exe', 'cert generate --install', 'C:\\proj', launcherPath) + ); + } + }); + + it('fails closed when a non-elevated process only has an unusable CLI path', () => { + assert.deepEqual( + decideElevatedWinappCommand(false, false, 'winapp', 'cert generate --install', 'C:\\proj', launcherPath), + { kind: 'error-cli-missing' } + ); + }); +}); + diff --git a/src/winapp-cli-utils.ts b/src/winapp-cli-utils.ts index 063c836..c952db9 100644 --- a/src/winapp-cli-utils.ts +++ b/src/winapp-cli-utils.ts @@ -41,6 +41,38 @@ export function escapePowerShellArg(value: string): string { return `'${value.replace(/'/g, "''")}'`; } +export function getWindowsPowerShellPath(systemRoot?: string): string { + return path.join(systemRoot || 'C:\\Windows', 'System32', 'WindowsPowerShell', 'v1.0', 'powershell.exe'); +} + +export function isUsableElevatedCliPath(cliPath: string, cliPathExists: boolean): boolean { + return path.isAbsolute(cliPath) && path.basename(cliPath).toLowerCase() === 'winapp.exe' && cliPathExists; +} + +export type ElevatedWinappCommandDecision = + | { kind: 'run-normally' } + | { kind: 'run-elevated'; command: string } + | { kind: 'error-cli-missing' }; + +export function decideElevatedWinappCommand( + isElevated: boolean, + cliPathIsUsable: boolean, + cliPath: string, + cliArgs: string, + workingDirectory: string, + launcherPath: string +): ElevatedWinappCommandDecision { + if (isElevated) { + return { kind: 'run-normally' }; + } + + if (!cliPathIsUsable) { + return { kind: 'error-cli-missing' }; + } + + return { kind: 'run-elevated', command: buildElevatedTerminalCommand(cliPath, cliArgs, workingDirectory, launcherPath) }; +} + /** * Build a PowerShell command that runs the winapp CLI elevated in a *separate* * console via `Start-Process -Verb RunAs`. @@ -63,13 +95,14 @@ export function escapePowerShellArg(value: string): string { * {@link escapePowerShellArg}, so values containing single quotes stay balanced * and cannot break out of the literal. * + * @param launcherPath Absolute path to Windows PowerShell for the elevated launcher. * @param cliPath Path to the bundled winapp executable. * @param cliArgs The winapp arguments, already PowerShell-escaped where needed * (e.g. `cert install 'C:\path\devcert.pfx'`). * @param workingDirectory Directory to run the elevated command in. * @returns A PowerShell command line to send to a (non-elevated) terminal. */ -export function buildElevatedTerminalCommand(cliPath: string, cliArgs: string, workingDirectory: string): string { +export function buildElevatedTerminalCommand(cliPath: string, cliArgs: string, workingDirectory: string, launcherPath: string): string { const innerCommand = `Set-Location -LiteralPath ${escapePowerShellArg(workingDirectory)}; & ${escapePowerShellArg(cliPath)} ${cliArgs}`.trim(); - return `Start-Process -FilePath 'powershell.exe' -Verb RunAs -ArgumentList '-NoExit', '-Command', ${escapePowerShellArg(innerCommand)}`; + return `Start-Process -FilePath ${escapePowerShellArg(launcherPath)} -Verb RunAs -ArgumentList '-NoExit', '-Command', ${escapePowerShellArg(innerCommand)}`; } From fe6ea01419cff685ddccc287ce252f02de9bd141 Mon Sep 17 00:00:00 2001 From: Chiara Mooney <34109996+chiaramooney@users.noreply.github.com> Date: Wed, 15 Jul 2026 14:15:50 -0700 Subject: [PATCH 3/4] Address PR review round 2: validate CLI path for elevated runs and clarify launcher terminal (#34) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5ce7415c-8966-43ec-ad27-59a68e7ed74b --- src/extension.ts | 2 +- src/test/shell-escape.test.ts | 9 ++++++++- src/winapp-cli-utils.ts | 8 ++++---- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/extension.ts b/src/extension.ts index 4051ebd..d5a0384 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -140,7 +140,7 @@ async function runWinappCommandElevated(extensionPath: string, command: string, } const terminal = vscode.window.createTerminal({ - name: 'WinApp CLI (Admin)', + name: 'WinApp CLI (Admin launcher)', cwd: cwd, shellPath: 'powershell.exe', env: { WINAPP_CLI_CALLER: WINAPP_CLI_CALLER_VALUE } diff --git a/src/test/shell-escape.test.ts b/src/test/shell-escape.test.ts index 19a8fb8..3d94b57 100644 --- a/src/test/shell-escape.test.ts +++ b/src/test/shell-escape.test.ts @@ -129,9 +129,16 @@ describe('isUsableElevatedCliPath', () => { }); describe('decideElevatedWinappCommand', () => { - it('routes elevated processes to the normal terminal without requiring an elevated-safe CLI path', () => { + it('fails closed when an elevated process only has an unusable CLI path', () => { assert.deepEqual( decideElevatedWinappCommand(true, false, 'winapp', 'cert generate --install', 'C:\\proj', launcherPath), + { kind: 'error-cli-missing' } + ); + }); + + it('routes elevated processes with a usable CLI path to the normal terminal', () => { + assert.deepEqual( + decideElevatedWinappCommand(true, true, 'C:\\ext\\bin\\winapp.exe', 'cert generate --install', 'C:\\proj', launcherPath), { kind: 'run-normally' } ); }); diff --git a/src/winapp-cli-utils.ts b/src/winapp-cli-utils.ts index c952db9..9ae3111 100644 --- a/src/winapp-cli-utils.ts +++ b/src/winapp-cli-utils.ts @@ -62,14 +62,14 @@ export function decideElevatedWinappCommand( workingDirectory: string, launcherPath: string ): ElevatedWinappCommandDecision { - if (isElevated) { - return { kind: 'run-normally' }; - } - if (!cliPathIsUsable) { return { kind: 'error-cli-missing' }; } + if (isElevated) { + return { kind: 'run-normally' }; + } + return { kind: 'run-elevated', command: buildElevatedTerminalCommand(cliPath, cliArgs, workingDirectory, launcherPath) }; } From 91ae9fd8d48a449ff0fc645d0a214c0d5c228777 Mon Sep 17 00:00:00 2001 From: Chiara Mooney <34109996+chiaramooney@users.noreply.github.com> Date: Wed, 15 Jul 2026 14:22:29 -0700 Subject: [PATCH 4/4] Address PR review round 3: use absolute PowerShell path for elevation probe and launcher terminal (#34) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5ce7415c-8966-43ec-ad27-59a68e7ed74b --- src/extension.ts | 9 +++++---- src/test/shell-escape.test.ts | 12 ++++++++---- src/winapp-cli-utils.ts | 2 +- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/extension.ts b/src/extension.ts index d5a0384..2e480d9 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -6,7 +6,7 @@ import { getWinappCliPath, WINAPP_CLI_CALLER_VALUE, escapePowerShellArg, - getWindowsPowerShellPath, + resolveWindowsPowerShellPath, isUsableElevatedCliPath, decideElevatedWinappCommand } from './winapp-cli-utils'; @@ -16,6 +16,7 @@ import { glob } from 'glob'; import { ManifestEditorProvider } from './manifest-editor/manifest-editor-provider'; const WINAPP_DEBUG_TYPE = 'winapp'; +const WINDOWS_POWERSHELL_PATH = resolveWindowsPowerShellPath(process.env.SystemRoot); /** * Maps debugger types to the VS Code extensions that provide them. @@ -91,7 +92,7 @@ async function runWinappCommand(extensionPath: string, command: string, cwd: str async function isProcessElevated(): Promise { return new Promise((resolve) => { execFile( - 'powershell.exe', + WINDOWS_POWERSHELL_PATH, [ '-NoProfile', '-NonInteractive', @@ -117,7 +118,7 @@ async function isProcessElevated(): Promise { async function runWinappCommandElevated(extensionPath: string, command: string, cwd: string): Promise { const isElevated = await isProcessElevated(); const cliPath = getWinappCliPath(extensionPath); - const launcherPath = getWindowsPowerShellPath(process.env.SystemRoot); + const launcherPath = WINDOWS_POWERSHELL_PATH; const decision = decideElevatedWinappCommand( isElevated, isUsableElevatedCliPath(cliPath, fs.existsSync(cliPath)), @@ -142,7 +143,7 @@ async function runWinappCommandElevated(extensionPath: string, command: string, const terminal = vscode.window.createTerminal({ name: 'WinApp CLI (Admin launcher)', cwd: cwd, - shellPath: 'powershell.exe', + shellPath: WINDOWS_POWERSHELL_PATH, env: { WINAPP_CLI_CALLER: WINAPP_CLI_CALLER_VALUE } }); terminal.show(); diff --git a/src/test/shell-escape.test.ts b/src/test/shell-escape.test.ts index 3d94b57..9c03034 100644 --- a/src/test/shell-escape.test.ts +++ b/src/test/shell-escape.test.ts @@ -3,7 +3,7 @@ import assert from 'node:assert/strict'; import { escapePowerShellArg, buildElevatedTerminalCommand, - getWindowsPowerShellPath, + resolveWindowsPowerShellPath, isUsableElevatedCliPath, decideElevatedWinappCommand } from '../winapp-cli-utils'; @@ -100,13 +100,17 @@ describe('buildElevatedTerminalCommand', () => { }); }); -describe('getWindowsPowerShellPath', () => { +describe('resolveWindowsPowerShellPath', () => { it('builds the launcher path from SystemRoot', () => { - assert.equal(getWindowsPowerShellPath('D:\\Windows'), 'D:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe'); + assert.equal(resolveWindowsPowerShellPath('D:\\Windows'), 'D:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe'); }); it('falls back to C:\\Windows when SystemRoot is unavailable', () => { - assert.equal(getWindowsPowerShellPath(), launcherPath); + assert.equal(resolveWindowsPowerShellPath(undefined), launcherPath); + }); + + it('falls back to C:\\Windows when SystemRoot is empty', () => { + assert.equal(resolveWindowsPowerShellPath(''), launcherPath); }); }); diff --git a/src/winapp-cli-utils.ts b/src/winapp-cli-utils.ts index 9ae3111..340fd04 100644 --- a/src/winapp-cli-utils.ts +++ b/src/winapp-cli-utils.ts @@ -41,7 +41,7 @@ export function escapePowerShellArg(value: string): string { return `'${value.replace(/'/g, "''")}'`; } -export function getWindowsPowerShellPath(systemRoot?: string): string { +export function resolveWindowsPowerShellPath(systemRoot: string | undefined): string { return path.join(systemRoot || 'C:\\Windows', 'System32', 'WindowsPowerShell', 'v1.0', 'powershell.exe'); }