Skip to content

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

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?

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions