microsoft / microsoft/WinAppVSCE

`winapp sign`: support `--password` and `--timestamp`, and move off terminal command strings

Open
#246 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
TypeScript
Stars
13
Forks
3
Avg merge
6d 1h
Merged PRs (30d)
11

Description

Problem

The extension's sign flow targets winappcli ~0.3.1, but scripts/download-cli.ps1 downloads latest — currently 0.6.1. The 0.6.1 surface is:

winapp sign <file-path> <cert-path> [--password <password>] [--timestamp <url>]

The extension supports neither option:

  • src/sign-utils.ts buildSignCommand() returns a PowerShell-escaped string: sign <file> <cert>.
  • signPackage() (src/extension.ts ~607) wires it through runWinappCommand, which does terminal.sendText('& <cli> ' + command) in an integrated terminal.

Consequences:

  1. Password-protected PFX is unusable. The CLI's default password is literally password, so extension-generated dev certs work by accident and every real cert fails.
  2. --timestamp is unreachable, so anything signed via the extension stops validating once the signing cert expires. The CLI help calls this out explicitly.
  3. The transport is wrong for a secret. terminal.sendText writes the command into shell history and the visible terminal buffer. The codebase already has the correct precedent: winapp.certInfo (src/extension.ts ~1670) uses spawn with an args array and shell: false, with an explicit comment that this is to keep the password out of terminal history and prevent argument injection. winapp.pack likewise uses args arrays via runWinappCapture.

Proposal

1. Transport: move sign onto runWinappCapture (args array, shell: false)

Replace runWinappCommand on the sign path with the existing runWinappCapture(extensionPath, args, cwd, progressTitle). It already spawns with shell: false and an argv array (no PowerShell parsing, no injection, no history), streams combined stdout/stderr to the shared WinApp output channel, shows a cancellable progress notification, and resolves with { code, output } so we can show a real success/failure notification instead of silently leaving a terminal open.

User-visible output is preserved, not lost — it moves from a terminal tab to the WinApp output channel, which is where pack output already goes. This also makes the post-pack "Sign" action stop switching the user from output channel to terminal mid-workflow.

Rejected alternative: runWinappTool / vscode.ProcessExecution. It preserves argument boundaries and a terminal, but VS Code echoes the resolved command line into the task terminal — which would print the password. Disqualified.

Required fix in runWinappCapture: it currently logs > winapp ${args.join(' ')} to the output channel, which would leak the password. Add an optional redaction hook (e.g. redactArgs?: (args: string[]) => string[]) so sign logs --password ***. Also worth verifying manually that winapp sign --verbose doesn't echo the password on stdout.

buildSignCommand signature change
export interface SignCommandOptions {
  filePath: string;
  certPath: string;
  password?: string;   // omitted → CLI default
  timestamp?: string;  // omitted → no timestamping
}

export function buildSignArgs(options: SignCommandOptions): string[];
  • No PowerShell escaping — raw values; escaping is the shell's job and there is no shell. escapePowerShellArg drops out of sign-utils.ts.
  • SignFlowResult.commandExecuted: string | undefinedargsExecuted: string[] | undefined, with a redaction helper so logs/tests never see the raw password.
  • SignFlowAdapter.runSignCommand(extensionPath, command, workspacePath)runSignCommand(extensionPath, args: string[], workspacePath).
2. Password handling — prompt with smart skip, no persistence by default

Three-layer resolution, first match wins:

  1. Known dev cert → skip the prompt. If the chosen cert is one the extension itself generated (winapp pack --generate-cert / winapp cert generate), its password is the CLI default. Detect this by recording generated cert paths in context.workspaceState when we run those commands, rather than guessing from the filename. If matched, pass no --password. This keeps today's zero-friction dev loop unchanged.
  2. SecretStorage lookup, keyed by absolute cert path. If a password was previously saved for this cert, reuse it silently.
  3. Prompt via showInputBox({ password: true, ignoreFocusOut: true }). Empty input = "use CLI default". After a successful sign, offer "Remember this password for this certificate?" → writes to vscode.SecretStorage (OS credential manager), never to settings.

Explicitly rejected: a settings-based password. settings.json is plaintext, frequently committed, and synced. If a CI-ish escape hatch is wanted, support an environment variable indirection (a setting naming an env var to read), not the secret itself.

On a wrong-password failure, detect the failing exit, purge the cached SecretStorage entry, and re-prompt once — otherwise a stale saved password becomes a permanently broken sign command with a confusing error.

3. --timestamp — setting-driven, on by default

A per-sign prompt is unacceptable friction.

  • winapp.sign.timestampServer (string), default http://timestamp.digicert.com, scope: "resource" so a workspace can override.
  • winapp.sign.timestamp (enum) — "always" (default) / "never". Default always because an untimestamped signature is a latent correctness bug and timestamping a dev build costs one network call.
  • If the timestamp server is unreachable the CLI will fail; surface a "timestamping failed — retry without timestamp?" action so an offline dev isn't hard-blocked.

Rejected: a "this is a production build" toggle in the QuickPick. It makes the safe behavior opt-in and the unsafe behavior the default, and adds a step to every sign.

4. Flow shape — stay linear, add an optional "Advanced" affordance

Happy path stays linear:

pick file → pick cert → [password prompt only if needed] → sign

With items 2 and 3 above, the common dev case (extension-generated cert, timestamp from setting) adds zero new prompts versus today.

For overrides, do not add a mandatory "Advanced options" step. Put a $(gear) "Advanced options…" item at the bottom of the certificate QuickPick, opening a secondary QuickPick with per-run toggles: override timestamp server, disable timestamping for this run, force a password re-prompt.

Add resolution logic behind the existing adapter pattern so it is unit-testable without VS Code:

export interface SignFlowAdapter {
  pickSignableFile(workspacePath: string): Promise<string | undefined>;
  pickCertificateFile(workspacePath: string): Promise<string | undefined>;
  resolveSignOptions(certPath: string): Promise<SignOptions | undefined>; // new
  runSignCommand(extensionPath: string, args: string[], workspacePath: string): Promise<number | null>;
  rememberPassword?(certPath: string, password: string): Promise<void>;
}

resolveSignOptions returning undefined = user cancelled → abort, matching existing cancellation semantics.

Interaction with handlePackCompletion

handlePackCompletion (src/extension.ts ~640) offers Reveal / Sign / Install after pack, and Sign calls signPackage(..., plan.artifactPath).

Nuance: when the user answered Yes to "Generate and install a development certificate?", winapp pack --generate-cert already signed the package (the CLI auto-signs when a cert is present), so offering "Sign" is redundant at best.

  • Capture the generated cert path from pack output and feed it into the sign flow as a prefilled cert, skipping the cert picker and the password prompt. Extend executeSignFlow with an optional prefilledCertPath, mirroring the existing prefilled-file-path pattern.
  • Consider suppressing or relabelling the Sign action when the artifact is already signed (flagging only; may be out of scope).

Test impact

  • src/test/sign-utils.test.ts — the four buildSignCommand cases (~L304–325) become deepEqual array assertions. The "paths with spaces" / "escapes quotes" cases get stronger: they assert verbatim pass-through, which is the real contract once there's no shell.
  • src/test/sign-flow.test.ts — ~6 assertions on commandExecuted substrings need updating for the argv array and the new adapter shape.
  • src/test/e2e/sign-quickpick.spec.ts — the two cancellation tests assert terminalCount === 0 (~L333, ~L449). Those still pass but become vacuous, since sign never creates a terminal under the new transport. Retarget them at "the WinApp output channel received no sign invocation", or at minimum fix the comments. Success-path tests are structurally unaffected, but a new password/advanced-options step changes the QuickPick sequence they drive.

Coordination with winapp az-sign

A separate effort is adding winapp az-sign (Azure Trusted Signing). Shared surface that should be agreed before either lands:

  1. A single "Sign File" entry point. winapp.sign should become a method chooser — "Certificate file (PFX)" vs "Azure Trusted Signing" — rather than two palette commands, with a winapp.sign.defaultMethod setting to skip the chooser.
  2. handlePackCompletion's Sign action must route into that chooser, not hardcode PFX signing.
  3. Shared transport. Both need args-array spawn with redacted logging; az-sign has credentials too. The redaction hook should be designed for both.
  4. Shared timestamp settings if az-sign accepts a timestamp option.
  5. Shared secret policy — neither method puts a secret in settings.json; both use SecretStorage.

Task breakdown

  • Add a redaction hook to runWinappCapture so --password is logged as ***
  • Replace buildSignCommand with buildSignArgs returning a raw argv array
  • Add SignOptions + resolveSignOptions to SignFlowAdapter
  • Wire vscode.SecretStorage for per-cert-path passwords (opt-in save, purge + re-prompt on auth failure)
  • Record extension-generated cert paths in workspaceState to skip the password prompt
  • Add winapp.sign.timestampServer and winapp.sign.timestamp settings to package.json
  • Add the $(gear) "Advanced options…" affordance to the certificate QuickPick
  • Migrate signPackage / executeSignFlow onto runWinappCapture with success/failure notifications
  • Prefill the pack-generated cert into the post-pack Sign action
  • Update sign-utils.test.ts, sign-flow.test.ts, and sign-quickpick.spec.ts
  • Coordinate the shared signing-method chooser with the az-sign work

Open questions

  1. Password strategy: prompt + opt-in SecretStorage (proposed), prompt every time with no persistence, or something else?
  2. Timestamp default: always-on with a default DigiCert server (proposed), or off by default?
  3. Is moving sign output from a terminal tab to the WinApp output channel acceptable, or is a visible terminal a requirement?
  4. Should winapp.sign become a method chooser shared with az-sign?

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with src/sign-utils.ts, signPackage and executeSignFlow in src/extension.ts, and the existing runWinappCapture, winapp.certInfo, and winapp.pack paths. Read the sign-utils, sign-flow, and sign-quickpick tests before resolving the open questions and coordinating with winapp az-sign. Done means argv-based signing, protected password handling, timestamp settings, redacted output, updated pack completion behavior, and passing tests.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript, vscode
Domain
cli, devtools, security
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.