From 2f416333064bfb108f1733f808642b7a8dcd605f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elberte=20Pl=C3=ADnio?= Date: Thu, 23 Jul 2026 02:16:54 -0300 Subject: [PATCH 1/2] feat(cli): route privileged provisioning through graphical sudo (askpass) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements the "Shared graphical sudo (askpass) security contract — locked v1" (pickforge/picklab#27, shared with pickforge/pickforge#215/#258): a pure four-state capability detector (available/no-helper/headless/ unsupported-platform) gated to Linux, wired into the provisioning executor so privileged steps run `sudo -A` with an explicit SUDO_ASKPASS — the only env var this feature injects — instead of the prior ad hoc `sudo -n`. Missing capability fails preflight, before any mutation, with an actionable error naming the manual `sudo ...` fallback; sudo-level denial/cancellation at runtime (detected from sudo's own `sudo:`-prefixed diagnostics, never prompt text) surfaces as a distinct `cancelled` status with no retry. Reconciled with the existing no-sudo policy: MCP tools still never import provisioning code or touch sudo (unchanged, still enforced by test/security/security-no-sudo.test.ts); the CLI's privileged-step path remains the sole, explicit, consent-gated sudo entry point. Also fixes a section-level privilegeUnavailable.reason override (written only for the pre-existing "sudo missing" case) from masking the new, correct askpass-unavailable message with a stale "sudo not found" one. --- README.md | 1 + docs/releases/UNRELEASED.md | 46 +++++ packages/cli/src/commands/doctor.ts | 3 +- packages/cli/src/commands/init.ts | 3 +- packages/cli/src/commands/setup-lab-user.ts | 3 +- packages/cli/src/provision/askpass.ts | 157 +++++++++++++++ packages/cli/src/provision/executor.ts | 115 +++++++++-- packages/cli/test/askpass.test.ts | 144 ++++++++++++++ packages/cli/test/cli.test.ts | 158 +++++++++++++-- packages/cli/test/executor.test.ts | 206 +++++++++++++++++++- 10 files changed, 799 insertions(+), 37 deletions(-) create mode 100644 packages/cli/src/provision/askpass.ts create mode 100644 packages/cli/test/askpass.test.ts diff --git a/README.md b/README.md index 6f35332..4763b18 100644 --- a/README.md +++ b/README.md @@ -304,6 +304,7 @@ A TypeScript monorepo. `@pickforge/picklab` is the published package; the rest a ## Security model - MCP tools never invoke sudo. Privileged provisioning happens only through the CLI (`picklab setup lab-user`, or `init` with explicit `--create-lab-user`), with explicit consent (`--yes` or a prompt). +- Privileged provisioning commands run through graphical `sudo` (`sudo -A`) on Linux, never a plain terminal password prompt: PickLab detects a graphical session (`WAYLAND_DISPLAY`/`DISPLAY`) and a `SUDO_ASKPASS` helper (your own `SUDO_ASKPASS`, or the first of `ksshaskpass`/`ssh-askpass`/`lxqt-openssh-askpass`/the standard distro paths) before spawning anything privileged, and injects `SUDO_ASKPASS` — the only environment variable this feature ever adds — into that one command. PickLab never ships, generates, or installs its own askpass helper, and never captures, logs, or persists the password prompt. macOS/Windows are out of scope for this release: no graphical prompt is attempted there. If no graphical session or helper is available (headless, SSH, CI, or a missing helper), or the platform isn't Linux, the command fails closed with an actionable error naming the manual fallback — run the same command yourself with `sudo` in a terminal. A cancelled or denied graphical prompt surfaces as a distinct failure with no automatic retry, and nothing about the prompt is written to logs, config, or run artifacts. - All user inputs are spawned as argument arrays — never interpolated into shell strings. - The DevTools relay validates the installed upstream package name, exact version, declared bin, and confined real path before spawning Node with an argument array. Its browser URL is always derived as `http://127.0.0.1:`. - Relay stdout is protocol-only. A pending JSON-RPC record is capped at 16 MiB. Upstream diagnostic lines are capped at 64 KiB, redacted, and forwarded only to stderr; an over-limit line is dropped with a safe notice. Upstream update checks and usage statistics are disabled. diff --git a/docs/releases/UNRELEASED.md b/docs/releases/UNRELEASED.md index 39950a4..1065f18 100644 --- a/docs/releases/UNRELEASED.md +++ b/docs/releases/UNRELEASED.md @@ -5,6 +5,21 @@ release description, then reset it after the release is published. ## User-facing changes +- **Privileged provisioning now runs through graphical `sudo` on Linux + (`picklab setup lab-user`, `picklab init --create-lab-user`, + `picklab doctor --fix`).** Instead of a plain terminal password prompt, + PickLab detects a graphical session (`WAYLAND_DISPLAY`/`DISPLAY`) and a + `SUDO_ASKPASS` helper (your own `SUDO_ASKPASS`, or the first of + `ksshaskpass`/`ssh-askpass`/`lxqt-openssh-askpass`/the standard distro + paths) before spawning anything privileged, then runs `sudo -A` with that + helper — the only environment variable this feature injects. PickLab never + ships or installs its own helper, and never captures, logs, or persists the + password prompt. Headless sessions, a missing helper, or a non-Linux + platform (macOS/Windows are out of scope this release) fail closed with an + actionable error naming the manual `sudo ...` fallback — no automatic + fallback to a plain interactive password prompt. A cancelled or denied + graphical prompt surfaces as a distinct failure with no retry. See the + README's "Security model" section. (pickforge/picklab#27) - **Default run storage changed.** Screenshots, logs, manifests, and evidence journals now default to `~/.pickforge/picklab/projects//runs/` (outside the project) instead of `/.picklab/runs/`, so a default @@ -75,6 +90,16 @@ release description, then reset it after the release is published. - Centralized CLI provisioning policy for plan classification, consent, dry-runs, ordered preflight failures/skips, adapter-owned sudo routing, cancellation, redacted presentation plans, and partial results. +- Added `packages/cli/src/provision/askpass.ts`, a pure four-state capability + detector (`available`/`no-helper`/`headless`/`unsupported-platform`) + implementing the "Shared graphical sudo (askpass) security contract — + locked v1" shared with pickforge/pickforge#215/#258 + (`crates/pickforge-core/src/process/askpass.rs`); wired it into the + provisioning executor's privileged-step materialization (`sudo -A` + + `SUDO_ASKPASS`, arg-array only) and into `setup lab-user`/`doctor + --fix`/`init`. sudo-level denial/cancellation (detected from sudo's own + `sudo:`-prefixed diagnostics, never prompt text) now surfaces as a distinct + `cancelled` executor status instead of a generic failure. - Added a framing-aware DevTools NDJSON relay with protocol validation, backpressure, bounded diagnostics, redacted failures, and evidence hooks. - Added atomic, crash-recoverable evidence journals, active-run ownership, @@ -101,6 +126,23 @@ release description, then reset it after the release is published. overrides) and `run-catalog.test.ts`'s "openRunCatalog storage modes" suite (home default, legacy project-local discovery, project isolation, custom mode), plus a `git status --porcelain` repo-cleanliness smoke test. +- New `askpass.test.ts` (capability detection: headless/graphical, user-set + vs. probe-list priority, empty-value handling, Linux/non-Linux gating) and + an extended `executor.test.ts` "privileged execution via graphical sudo" + suite (materialization to `sudo -A` + `SUDO_ASKPASS`, env-propagation + proving only that key is added, arg-array safety for hostile argv, + preflight failure for each non-available capability state, sudo + cancellation/denial via a stand-in `sudo` script surfacing a distinct + `cancelled` status with no retry, and redaction of a planted secret in a + sudo denial message). Extended `cli.test.ts` with real end-to-end coverage + routing `setup lab-user`/`init --create-lab-user` through a fake graphical + `sudo -A` + `SUDO_ASKPASS` helper, plus platform-appropriate manual-fallback + assertions for the no-session/no-helper/non-Linux cases. The + happy-path/cancellation `cli.test.ts` cases that need a real Linux + graphical-session capability are `skipIf(process.platform !== "linux")` + (this dev sandbox is macOS); they run in full on Linux CI. + Real `sudo -A -v` hardware smoke (a live graphical prompt, not a stand-in) + is not covered here and is deferred to a real Linux desktop. - `bun run test` — same pre-existing failure set as unmodified `main`, verified test-by-test against `main` on this branch's (macOS) dev sandbox. These are platform-environment failures, not specific to this change; on @@ -120,6 +162,10 @@ release description, then reset it after the release is published. Darwin-only failures above stop the process before the v8 coverage provider flushes it — reproduced identically on unmodified `main`). - Platform smoke checks outside Linux. +- Real `sudo -A -v` graphical-prompt smoke on a live Linux desktop (approve + and cancel) — the automated suite covers this with a stand-in `sudo`/ + askpass-path script per test, never a real graphical prompt; a real-desktop + smoke is deferred. - Live remote SSH-tunnel smoke test. - Tag-triggered npm publish and draft-release creation (runs only after merge and tag push). diff --git a/packages/cli/src/commands/doctor.ts b/packages/cli/src/commands/doctor.ts index 69a47d5..5c4e2a7 100644 --- a/packages/cli/src/commands/doctor.ts +++ b/packages/cli/src/commands/doctor.ts @@ -1,4 +1,5 @@ import type { EnvLike } from "@pickforge/picklab-core"; +import { resolveAskpassCapability } from "../provision/askpass.js"; import { evaluateChecks, formatCheckLine, @@ -169,7 +170,7 @@ export async function runDoctor( log, privilege: { sudoPath: snapshot.sudo, - nonInteractive: process.stdin.isTTY !== true, + askpass: resolveAskpassCapability(env), }, }, ); diff --git a/packages/cli/src/commands/init.ts b/packages/cli/src/commands/init.ts index 6b603ff..345ba57 100644 --- a/packages/cli/src/commands/init.ts +++ b/packages/cli/src/commands/init.ts @@ -1,5 +1,6 @@ import path from "node:path"; import type { EnvLike, PicklabProfile } from "@pickforge/picklab-core"; +import { resolveAskpassCapability } from "../provision/askpass.js"; import { evaluateChecks, formatCheckLine, @@ -202,7 +203,7 @@ export async function runInit( log, privilege: { sudoPath: snapshot.sudo, - nonInteractive: process.stdin.isTTY !== true, + askpass: resolveAskpassCapability(env), }, }, ); diff --git a/packages/cli/src/commands/setup-lab-user.ts b/packages/cli/src/commands/setup-lab-user.ts index b1d590f..e87fc99 100644 --- a/packages/cli/src/commands/setup-lab-user.ts +++ b/packages/cli/src/commands/setup-lab-user.ts @@ -1,4 +1,5 @@ import type { EnvLike } from "@pickforge/picklab-core"; +import { resolveAskpassCapability } from "../provision/askpass.js"; import { collectSnapshot } from "../provision/detect.js"; import { executeProvisioning, @@ -108,7 +109,7 @@ export async function runSetupLabUser( log, privilege: { sudoPath: snapshot.sudo, - nonInteractive: process.stdin.isTTY !== true, + askpass: resolveAskpassCapability(env), }, beforeExecute: () => { if (opts.json !== true && snapshot.labUser.exists) { diff --git a/packages/cli/src/provision/askpass.ts b/packages/cli/src/provision/askpass.ts new file mode 100644 index 0000000..569c316 --- /dev/null +++ b/packages/cli/src/provision/askpass.ts @@ -0,0 +1,157 @@ +// Linux graphical `sudo` (askpass) capability detection — +// pickforge/picklab#27. +// +// Implements the *detection* half of the "Shared graphical sudo (askpass) +// security contract — locked v1" pinned on that issue (shared with +// pickforge/pickforge#215, which ships the same semantics in +// crates/pickforge-core/src/process/askpass.rs): Linux graphical sessions +// only, a fixed probe list (no dynamic discovery, no bundled helper), and +// fail-closed when nothing resolves. The *injection* half — passing +// `SUDO_ASKPASS`, and only that variable, into the spawned privileged +// command — lives in ./executor.ts (materializePrivilegedStep), since +// PickLab programmatically issues `sudo -A` rather than hosting an +// interactive shell the way PickForge does. +// +// Scope: macOS/Windows are out of scope for this release. +// `detectAskpassCapability` is a pure function so every branch — including +// the user-set-vs-probe-list priority order — is directly testable on any +// host; only `resolveAskpassCapability` gates real filesystem probing to +// Linux, matching where the feature actually activates. + +import fs from "node:fs"; +import type { EnvLike } from "@pickforge/picklab-core"; + +/** + * Fixed, documented askpass helper probe list per the locked v1 contract — + * checked in this order after a user-set `SUDO_ASKPASS`. No dynamic + * discovery beyond this list; PickLab never ships, generates, or installs + * its own askpass helper. + */ +export const ASKPASS_PROBE_PATHS: readonly string[] = [ + "/usr/bin/ksshaskpass", + "/usr/bin/ssh-askpass", + "/usr/bin/lxqt-openssh-askpass", + "/usr/bin/ssh-askpass-gnome", + "/usr/lib/ssh/ssh-askpass", + "/usr/lib/openssh/gnome-ssh-askpass", + "/usr/lib/seahorse/ssh-askpass", +]; + +/** + * The result of the pre-flight capability check, run before any privileged + * step materializes. Every state is distinct and observable per the + * contract's failure semantics — callers must not collapse `no-helper` and + * `headless` into one generic "unavailable". + */ +export type AskpassCapability = + /** A graphical session and a resolvable helper were both found. `helper` + * is the absolute path to hand to `SUDO_ASKPASS`. */ + | { state: "available"; helper: string } + /** A graphical session was detected but no helper resolved: the user's + * `SUDO_ASKPASS` (if set) doesn't point at an executable file, and + * nothing on the fixed probe list exists either. */ + | { state: "no-helper" } + /** No graphical session (`WAYLAND_DISPLAY`/`DISPLAY` both unset or empty + * in the resolved environment) — SSH, a bare TTY, or headless CI. */ + | { state: "headless" } + /** This platform is out of scope for the locked v1 contract + * (macOS/Windows). Never `available` here — the feature is a documented + * no-op, not a half-implementation. */ + | { state: "unsupported-platform" }; + +function nonEmpty(value: string | undefined): value is string { + return value !== undefined && value !== ""; +} + +/** + * Resolve capability from an already-resolved environment (the caller must + * pass the environment the privileged command will actually inherit — see + * `resolveAskpassCapability`). Pure aside from the injected `isExecutable` + * probe, so every branch is directly testable without a real graphical + * session or real helper binaries on disk. + */ +export function detectAskpassCapability( + env: EnvLike, + isExecutable: (path: string) => boolean, +): AskpassCapability { + const graphical = nonEmpty(env.WAYLAND_DISPLAY) || nonEmpty(env.DISPLAY); + if (!graphical) { + return { state: "headless" }; + } + + // (1) user-set SUDO_ASKPASS, only if it points to an executable file. + const userSet = env.SUDO_ASKPASS; + if (nonEmpty(userSet) && isExecutable(userSet)) { + return { state: "available", helper: userSet }; + } + + // (2) first existing helper from the fixed probe list. No dynamic + // discovery beyond this list. + for (const candidate of ASKPASS_PROBE_PATHS) { + if (isExecutable(candidate)) { + return { state: "available", helper: candidate }; + } + } + + return { state: "no-helper" }; +} + +function isExecutableFile(target: string): boolean { + try { + const stat = fs.statSync(target); + return stat.isFile() && (stat.mode & 0o111) !== 0; + } catch { + return false; + } +} + +/** + * Resolve the current process's askpass capability. Gated to Linux per the + * locked v1 contract's scope — macOS/Windows are always + * `unsupported-platform`, never a half-implementation. + * + * Unlike PickForge's long-lived desktop process (which caches this once per + * process lifetime), the PickLab CLI is a short one-shot invocation that + * re-execs per command, so there is no stale-cache risk to guard against + * here: detection simply re-runs, cheaply, against the current environment + * each time a provisioning command resolves it. + */ +export function resolveAskpassCapability( + env: EnvLike = process.env, +): AskpassCapability { + if (process.platform !== "linux") { + return { state: "unsupported-platform" }; + } + return detectAskpassCapability(env, isExecutableFile); +} + +/** + * A human-actionable message for every non-available state. Always includes + * the manual fallback the locked v1 contract requires: run the privileged + * command yourself, in a terminal, with sudo — never an automatic fallback + * to interactive password capture. + */ +export function askpassUnavailableMessage( + capability: Exclude, + manualCommand: string, +): string { + const fallback = `Run it yourself in a terminal: ${manualCommand}`; + switch (capability.state) { + case "headless": + return ( + "No graphical session detected (WAYLAND_DISPLAY and DISPLAY are " + + `both unset); graphical sudo prompts require one. ${fallback}` + ); + case "no-helper": + return ( + "No SUDO_ASKPASS helper found (checked your SUDO_ASKPASS and the " + + `standard probe list: ${ASKPASS_PROBE_PATHS.join(", ")}). Install ` + + `one (e.g. ssh-askpass) or set SUDO_ASKPASS to point at one. ${fallback}` + ); + case "unsupported-platform": + return ( + "Graphical sudo prompts are only supported on Linux in this " + + `release (detected ${process.platform}). ${fallback}` + ); + } +} diff --git a/packages/cli/src/provision/executor.ts b/packages/cli/src/provision/executor.ts index f4e0101..20a1870 100644 --- a/packages/cli/src/provision/executor.ts +++ b/packages/cli/src/provision/executor.ts @@ -11,10 +11,28 @@ import { type EnvLike, type PicklabConfig, } from "@pickforge/picklab-core"; -import { formatStep, type ProvisioningPlan, type ProvisioningStep } from "./plan.js"; +import { + askpassUnavailableMessage, + type AskpassCapability, +} from "./askpass.js"; +import { + formatStep, + type CommandStep, + type ProvisioningPlan, + type ProvisioningStep, +} from "./plan.js"; const DEFAULT_STEP_TIMEOUT_MS = 180_000; +// Real `sudo` prefixes its own diagnostics with "sudo:" — auth failure, +// cancellation (the askpass helper produced no password), and "no askpass +// program specified" all take this shape. The wrapped command's own stderr +// does not, so this is the fail-closed line between "sudo denied/cancelled +// this" and "the privileged command itself failed" without ever parsing +// prompt text or credentials (sudo never echoes those to its own stderr — +// they go straight from the askpass helper into sudo's authentication path). +const SUDO_DENIAL_RE = /^sudo:/im; + export type PlanClassification = | "empty" | "automatic" @@ -71,7 +89,10 @@ export interface ExecuteProvisioningOptions { adapter?: ProvisioningExecutionAdapter; privilege?: { sudoPath: string | null; - nonInteractive?: boolean; + /** Pre-resolved via `resolveAskpassCapability` at command-invocation + * time (per the locked v1 contract). Missing is treated as `headless` — + * fail-closed, never an implicit "available". */ + askpass?: AskpassCapability; }; beforeExecute?: (plan: ProvisioningPlan) => void | Promise; } @@ -115,6 +136,28 @@ type PreparedSection = class PrivilegeUnavailableError extends Error {} +/** Preflight failure: the `sudo` binary itself isn't on PATH. Callers may + * override this reason with a section's static `privilegeUnavailable.reason` + * (e.g. `labUserPrivilegeUnavailableMessage`) — that override predates the + * askpass contract and was always written to describe exactly this case. */ +class SudoMissingError extends PrivilegeUnavailableError {} + +/** Preflight failure: sudo exists, but the locked v1 askpass contract's + * capability check didn't resolve to `available` (headless, no helper, or + * an unsupported platform). Its message is already the specific, + * state-correct, actionable one from `askpassUnavailableMessage` — a + * section's static `privilegeUnavailable.reason` (written only for the + * sudo-missing case above) must never paper over it with a stale, wrong + * "sudo not found" message. */ +class AskpassUnavailableError extends PrivilegeUnavailableError {} + +/** Thrown when a privileged command's own execution is denied or cancelled + * by sudo (auth failure, or the user dismissing the graphical prompt) — a + * distinct, actionable runtime state per the locked v1 contract, never + * retried automatically. Exported so custom adapters can raise the same + * state without re-implementing the `sudo:`-prefix heuristic. */ +export class PrivilegedCommandDeniedError extends Error {} + export function classifyPlan(plan: ProvisioningPlan): PlanClassification { if (plan.steps.length === 0) return "empty"; const consentSteps = plan.steps.filter( @@ -167,6 +210,14 @@ async function executeLocalStep( result.stderr.trim() || result.stdout.trim() || (result.timedOut ? "timed out" : `exit code ${result.code}`); + if (step.privileged && SUDO_DENIAL_RE.test(result.stderr)) { + throw new PrivilegedCommandDeniedError( + `sudo denied or cancelled this privileged command: ${detail}. ` + + "No changes were made by this step. Approve the graphical " + + "sudo prompt and re-run, or run it yourself in a terminal: " + + manualSudoCommandFromMaterialized(step), + ); + } throw new Error( `${command.cmd} ${command.args.join(" ")} failed: ${detail}`, ); @@ -187,27 +238,52 @@ async function executeLocalStep( } } +/** Reconstruct the command line a user can paste into a terminal themselves + * — the manual fallback the locked v1 contract requires whenever graphical + * sudo isn't available. For the raw (pre-materialization) step. */ +function manualSudoCommand(step: CommandStep): string { + return `sudo ${step.command.cmd} ${step.command.args.join(" ")}`.trim(); +} + +/** Same fallback, reconstructed from an already-materialized `sudo -A + * ` step (as seen inside `executeLocalStep`) by dropping the `-A` + * flag we added ourselves. Only ever called on steps this module + * materialized, so the `["-A", cmd, ...args]` shape is guaranteed. */ +function manualSudoCommandFromMaterialized(step: CommandStep): string { + return `sudo ${step.command.args.slice(1).join(" ")}`.trim(); +} + function materializePrivilegedStep( step: ProvisioningStep, opts: ExecuteProvisioningOptions, ): ProvisioningStep { const sudoPath = opts.privilege?.sudoPath; if (sudoPath === undefined || sudoPath === null) { - throw new PrivilegeUnavailableError("sudo not found on PATH"); + throw new SudoMissingError("sudo not found on PATH"); } if (step.kind !== "command") { throw new Error(`Unsupported privileged provisioning step: ${step.kind}`); } + // Missing capability is treated as `headless` — fail-closed, never an + // implicit "available" — so a caller that forgets to resolve+pass it + // cannot accidentally spawn sudo without a graphical prompt. + const capability: AskpassCapability = + opts.privilege?.askpass ?? { state: "headless" }; + if (capability.state !== "available") { + throw new AskpassUnavailableError( + askpassUnavailableMessage(capability, manualSudoCommand(step)), + ); + } return { ...step, command: { ...step.command, cmd: sudoPath, - args: [ - ...(opts.privilege?.nonInteractive === true ? ["-n"] : []), - step.command.cmd, - ...step.command.args, - ], + args: ["-A", step.command.cmd, ...step.command.args], + // SUDO_ASKPASS is the only environment variable this feature may + // inject (locked v1 contract) — every other key from the step's own + // `command.env` passes through untouched. + env: { ...step.command.env, SUDO_ASKPASS: capability.helper }, }, }; } @@ -268,6 +344,11 @@ function prepareSections( })), }; } catch (error) { + if (error instanceof AskpassUnavailableError) { + // Always the specific, state-correct reason — never masked by a + // section's static privilegeUnavailable.reason (see class doc). + return { kind: "unavailable", section, reason: error.message }; + } if (error instanceof PrivilegeUnavailableError) { return { kind: "unavailable", @@ -410,10 +491,22 @@ export async function executeProvisioning( results.push({ id: presentation.id, ok: true, detail: formatted }); } catch (error) { const detail = redactSecrets((error as Error).message); - const message = `Step "${presentation.id}" failed: ${detail}`; - log(`[failed] ${title}: ${detail}`); + // sudo denial/cancellation is a distinct, actionable runtime state + // (locked v1 contract) — never folded into a generic "failed" and + // never retried automatically. + const cancelledBySudo = error instanceof PrivilegedCommandDeniedError; + const message = `Step "${presentation.id}" ${ + cancelledBySudo ? "cancelled" : "failed" + }: ${detail}`; + log(`[${cancelledBySudo ? "cancelled" : "failed"}] ${title}: ${detail}`); results.push({ id: presentation.id, ok: false, detail: message }); - return executionResult("failed", selected, skipped, results, [message]); + return executionResult( + cancelledBySudo ? "cancelled" : "failed", + selected, + skipped, + results, + [message], + ); } } return executionResult("completed", selected, skipped, results, []); diff --git a/packages/cli/test/askpass.test.ts b/packages/cli/test/askpass.test.ts new file mode 100644 index 0000000..04251f2 --- /dev/null +++ b/packages/cli/test/askpass.test.ts @@ -0,0 +1,144 @@ +// pickforge/picklab#27 — Linux graphical sudo (askpass) capability +// detection. Mirrors the branch coverage of the shared Rust reference +// (crates/pickforge-core/src/process/askpass.rs in pickforge/pickforge#215) +// so both repos' implementations of the locked v1 contract stay provably in +// sync: headless/graphical, user-set-vs-probe-list priority, empty-value +// handling, and the four distinct states. + +import { describe, expect, it } from "vitest"; +import { + ASKPASS_PROBE_PATHS, + askpassUnavailableMessage, + detectAskpassCapability, + resolveAskpassCapability, +} from "../src/provision/askpass.js"; + +describe("detectAskpassCapability", () => { + it("is headless when no display vars are present", () => { + expect(detectAskpassCapability({ PATH: "/usr/bin" }, () => true)).toEqual( + { state: "headless" }, + ); + }); + + it("is headless when display vars are present but empty", () => { + expect( + detectAskpassCapability( + { WAYLAND_DISPLAY: "", DISPLAY: "" }, + () => true, + ), + ).toEqual({ state: "headless" }); + }); + + it("is graphical via WAYLAND_DISPLAY alone", () => { + expect( + detectAskpassCapability({ WAYLAND_DISPLAY: "wayland-0" }, () => true), + ).toEqual({ state: "available", helper: ASKPASS_PROBE_PATHS[0] }); + }); + + it("is graphical via DISPLAY alone", () => { + expect(detectAskpassCapability({ DISPLAY: ":0" }, () => true)).toEqual({ + state: "available", + helper: ASKPASS_PROBE_PATHS[0], + }); + }); + + it("prefers a user-set executable SUDO_ASKPASS over the probe list", () => { + const capability = detectAskpassCapability( + { DISPLAY: ":0", SUDO_ASKPASS: "/opt/custom/my-askpass" }, + (p) => p === "/opt/custom/my-askpass", + ); + expect(capability).toEqual({ + state: "available", + helper: "/opt/custom/my-askpass", + }); + }); + + it("falls back to the probe list when the user-set SUDO_ASKPASS is not executable", () => { + const capability = detectAskpassCapability( + { DISPLAY: ":0", SUDO_ASKPASS: "/opt/custom/not-there" }, + (p) => p === ASKPASS_PROBE_PATHS[1], + ); + expect(capability).toEqual({ + state: "available", + helper: ASKPASS_PROBE_PATHS[1], + }); + }); + + it("treats an empty user-set SUDO_ASKPASS as unset (probe list still gets a chance)", () => { + const capability = detectAskpassCapability( + { DISPLAY: ":0", SUDO_ASKPASS: "" }, + (p) => p === ASKPASS_PROBE_PATHS[0], + ); + expect(capability).toEqual({ + state: "available", + helper: ASKPASS_PROBE_PATHS[0], + }); + }); + + it("is no-helper when graphical but nothing on the probe list resolves", () => { + expect( + detectAskpassCapability( + { DISPLAY: ":0", SUDO_ASKPASS: "/nope" }, + () => false, + ), + ).toEqual({ state: "no-helper" }); + }); + + it("never surfaces an unvalidated environment value as a helper", () => { + // The only escape hatch for untrusted input is SUDO_ASKPASS, and it is + // used only after `isExecutable` accepts it — a value that fails + // validation can never reach `available`. + const capability = detectAskpassCapability( + { + DISPLAY: ":0", + SUDO_ASKPASS: "rm -rf / #not-a-path-and-not-executable", + }, + () => false, + ); + expect(capability).toEqual({ state: "no-helper" }); + }); +}); + +describe("resolveAskpassCapability", () => { + it("is unsupported-platform on non-Linux platforms regardless of env", () => { + const original = process.platform; + Object.defineProperty(process, "platform", { value: "darwin" }); + try { + expect(resolveAskpassCapability({ DISPLAY: ":0" })).toEqual({ + state: "unsupported-platform", + }); + } finally { + Object.defineProperty(process, "platform", { value: original }); + } + }); + + it("detects against the real filesystem on Linux", () => { + const original = process.platform; + Object.defineProperty(process, "platform", { value: "linux" }); + try { + // No graphical session vars set here -> headless, deterministically, + // without needing any real askpass binaries on the test machine. + expect(resolveAskpassCapability({})).toEqual({ state: "headless" }); + } finally { + Object.defineProperty(process, "platform", { value: original }); + } + }); +}); + +describe("askpassUnavailableMessage", () => { + const manual = "sudo useradd -r -M picklab-lab"; + + it.each([ + ["headless", { state: "headless" } as const, /graphical session/i], + ["no-helper", { state: "no-helper" } as const, /SUDO_ASKPASS helper/i], + [ + "unsupported-platform", + { state: "unsupported-platform" } as const, + /only supported on Linux/i, + ], + ])("names the manual fallback for %s", (_label, capability, expected) => { + const message = askpassUnavailableMessage(capability, manual); + expect(message).toMatch(expected); + expect(message).toContain(`Run it yourself in a terminal: ${manual}`); + }); +}); diff --git a/packages/cli/test/cli.test.ts b/packages/cli/test/cli.test.ts index f834054..768f49e 100644 --- a/packages/cli/test/cli.test.ts +++ b/packages/cli/test/cli.test.ts @@ -92,6 +92,12 @@ function makeFakeSdk(root: string, opts: FakeSdkOptions = {}): string { interface FakeEnvOptions { bins?: Record; sdk?: string; + /** Stand in for a real graphical session + resolvable askpass helper + * (locked v1 contract) so privileged steps materialize into `sudo -A` + * instead of failing preflight. PickLab never ships its own helper, so + * tests that need one point SUDO_ASKPASS at a fake executable — none of + * the fake `sudo` stand-ins below actually invoke it. */ + graphicalSudo?: boolean; } function makeEnv( @@ -114,6 +120,12 @@ function makeEnv( if (opts.sdk !== undefined) { env.ANDROID_HOME = opts.sdk; } + if (opts.graphicalSudo === true) { + const helper = path.join(bin, "fake-askpass"); + writeScript(helper, "exit 0"); + env.DISPLAY = ":0"; + env.SUDO_ASKPASS = helper; + } return env; } @@ -290,9 +302,16 @@ describe("picklab init", () => { expect(fs.existsSync(env.PICKLAB_HOME!)).toBe(false); }); - it("fails closed when --create-lab-user lacks --yes in a non-interactive session", async () => { + // Graphical sudo is Linux-only (locked v1 contract) — on any other + // platform the askpass preflight check always fails first, before the + // consent gate under test here even runs. Runs on Linux CI; skips + // harmlessly elsewhere (see also "picklab setup lab-user" below). + it.skipIf(process.platform !== "linux")( + "fails closed when --create-lab-user lacks --yes in a non-interactive session", + async () => { const sudoLog = path.join(tmpDir, "sudo.log"); const env = makeEnv(tmpDir, { + graphicalSudo: true, bins: { sudo: `printf '%s\\n' "$*" >> ${sudoLog}\nexit 0` }, }); const projectDir = path.join(tmpDir, "project"); @@ -380,7 +399,10 @@ describe("picklab init", () => { expect(fs.existsSync(sudoLog)).toBe(false); }); - it("plans lab-user sudo steps for desktop+android with explicit consent", async () => { + // Linux-only (see rationale above). + it.skipIf(process.platform !== "linux")( + "plans lab-user sudo steps for desktop+android with explicit consent", + async () => { const sudoLog = path.join(tmpDir, "sudo.log"); const sdk = makeFakeSdk(path.join(tmpDir, "sdk"), { images: [IMAGE], @@ -388,6 +410,7 @@ describe("picklab init", () => { }); const env = makeEnv(tmpDir, { sdk, + graphicalSudo: true, bins: { Xvfb: "exit 0", xdotool: "exit 0", @@ -425,7 +448,7 @@ describe("picklab init", () => { "persist-lab-user", ]); expect(plan.find((step) => step.id === "useradd").command.args).toEqual([ - "-n", + "-A", "useradd", "-r", "-M", @@ -528,6 +551,44 @@ describe("picklab init", () => { expect(fs.existsSync(env.PICKLAB_HOME!)).toBe(false); }); + it("fails closed with an actionable manual fallback when sudo exists but no graphical session is available", async () => { + const sudoLog = path.join(tmpDir, "sudo.log"); + const env = makeEnv(tmpDir, { + bins: { sudo: `printf '%s\\n' "$*" >> ${sudoLog}\nexit 0` }, + }); + const projectDir = path.join(tmpDir, "project"); + fs.mkdirSync(projectDir); + const result = await runCli( + [ + "init", + "--profile", + "generic", + "--yes", + "--create-lab-user", + "--json", + ], + env, + projectDir, + ); + expect(result.code).toBe(1); + const report = parseJson(result); + const errors = (report.errors as string[]).join("\n"); + // Linux CI hits the "no graphical session" branch (this env sets no + // DISPLAY/WAYLAND_DISPLAY); any other dev platform hits the always-Linux + // -only branch first (locked v1 contract scope) — both name the same + // manual fallback, which is what this test actually verifies end to end. + expect(errors).toContain( + process.platform === "linux" + ? "No graphical session detected" + : "only supported on Linux", + ); + expect(errors).toContain( + "Run it yourself in a terminal: sudo useradd -r -M -s /usr/sbin/nologin picklab-lab", + ); + expect(fs.existsSync(sudoLog)).toBe(false); + expect(fs.existsSync(path.join(projectDir, ".picklab"))).toBe(false); + }); + it("retains earlier AVD sections when later lab-user privilege is unavailable", async () => { const sdk = makeFakeSdk(path.join(tmpDir, "sdk"), { images: [IMAGE] }); const env = makeEnv(tmpDir, { sdk }); @@ -586,11 +647,15 @@ describe("picklab init", () => { expect(fs.existsSync(env.PICKLAB_HOME!)).toBe(false); }); - it("materializes selected sudo steps even when another section blocks execution", async () => { + // Linux-only (see rationale above). + it.skipIf(process.platform !== "linux")( + "materializes selected sudo steps even when another section blocks execution", + async () => { const sudoLog = path.join(tmpDir, "sudo.log"); const sdk = makeFakeSdk(path.join(tmpDir, "sdk")); const env = makeEnv(tmpDir, { sdk, + graphicalSudo: true, bins: { sudo: `printf '%s\\n' "$*" >> ${sudoLog}\nexit 0` }, }); const projectDir = path.join(tmpDir, "project"); @@ -613,7 +678,7 @@ describe("picklab init", () => { const useradd = report.plan.find( (step: { id: string }) => step.id === "useradd", ); - expect(useradd.command.args.slice(0, 2)).toEqual(["-n", "useradd"]); + expect(useradd.command.args.slice(0, 2)).toEqual(["-A", "useradd"]); expect(fs.existsSync(sudoLog)).toBe(false); }); @@ -683,7 +748,10 @@ describe("picklab init", () => { expect(globalConfig.android.avdName).toBe("picklab-avd"); }); - it("preserves unprivileged and privileged step order in one init", async () => { + // Linux-only (see rationale above). + it.skipIf(process.platform !== "linux")( + "preserves unprivileged and privileged step order in one init", + async () => { const sequenceLog = path.join(tmpDir, "sequence.log"); const sdk = makeFakeSdk(path.join(tmpDir, "sdk"), { images: [IMAGE] }); writeScript( @@ -692,6 +760,7 @@ describe("picklab init", () => { ); const env = makeEnv(tmpDir, { sdk, + graphicalSudo: true, bins: { sudo: `echo "sudo:$*" >> ${sequenceLog}\nexit 0` }, }); const projectDir = path.join(tmpDir, "project"); @@ -714,16 +783,24 @@ describe("picklab init", () => { expect(result.code).toBe(0); const sequence = fs.readFileSync(sequenceLog, "utf8").trim().split("\n"); expect(sequence[0]).toBe("avdmanager"); - expect(sequence.slice(1).every((line) => line.startsWith("sudo:-n "))).toBe( + expect(sequence.slice(1).every((line) => line.startsWith("sudo:-A "))).toBe( true, ); }); }); describe("picklab setup lab-user", () => { - it("prints the provisioning plan in --dry-run without running sudo", async () => { + // Graphical sudo is Linux-only (locked v1 contract): resolveAskpassCapability + // gates on process.platform before even looking at DISPLAY/SUDO_ASKPASS, so + // any test that needs the "available" happy path to actually route a step + // through sudo can only run for real on Linux. These skip harmlessly on + // other dev platforms and run in full on Linux CI. + it.skipIf(process.platform !== "linux")( + "prints the provisioning plan in --dry-run without running sudo", + async () => { const sudoLog = path.join(tmpDir, "sudo.log"); const env = makeEnv(tmpDir, { + graphicalSudo: true, bins: { sudo: `printf '%s\\n' "$*" >> ${sudoLog}\nexit 0` }, }); const result = await runCli( @@ -743,7 +820,7 @@ describe("picklab setup lab-user", () => { "persist-lab-user", ]); expect((report.plan as Array)[0].command.args).toEqual([ - "-n", + "-A", "useradd", "-r", "-M", @@ -754,14 +831,20 @@ describe("picklab setup lab-user", () => { expect(fs.existsSync(sudoLog)).toBe(false); }); - it("fails closed without --yes in a non-interactive session", async () => { - const env = makeEnv(tmpDir, { bins: { sudo: "exit 0" } }); + // Linux-only (see rationale above). + it.skipIf(process.platform !== "linux")( + "fails closed without --yes in a non-interactive session", + async () => { + const env = makeEnv(tmpDir, { + graphicalSudo: true, + bins: { sudo: "exit 0" }, + }); const result = await runCli(["setup", "lab-user", "--json"], env, tmpDir); expect(result.code).toBe(1); const report = parseJson(result); expect((report.errors as string[]).join("\n")).toContain("--yes"); expect((report.plan as Array)[0].command.args).toEqual([ - "-n", + "-A", "useradd", "-r", "-M", @@ -771,6 +854,31 @@ describe("picklab setup lab-user", () => { ]); }); + it("fails closed with an actionable manual fallback when no graphical session is available", async () => { + const sudoLog = path.join(tmpDir, "sudo.log"); + const env = makeEnv(tmpDir, { + bins: { sudo: `printf '%s\\n' "$*" >> ${sudoLog}\nexit 0` }, + }); + const result = await runCli( + ["setup", "lab-user", "--yes", "--json"], + env, + tmpDir, + ); + expect(result.code).toBe(1); + const report = parseJson(result); + const errors = (report.errors as string[]).join("\n"); + // See the equivalent "picklab init" test above for why this branches. + expect(errors).toContain( + process.platform === "linux" + ? "No graphical session detected" + : "only supported on Linux", + ); + expect(errors).toContain( + "Run it yourself in a terminal: sudo useradd -r -M -s /usr/sbin/nologin picklab-lab", + ); + expect(fs.existsSync(sudoLog)).toBe(false); + }); + it("does not print the already-existing user before consent", async () => { const home = path.join(tmpDir, "missing-home"); const env = makeEnv(tmpDir, { @@ -789,9 +897,13 @@ describe("picklab setup lab-user", () => { expect(fs.existsSync(home)).toBe(false); }); - it("prints the already-existing user before approved dry-run logs", async () => { + // Linux-only (see rationale above). + it.skipIf(process.platform !== "linux")( + "prints the already-existing user before approved dry-run logs", + async () => { const home = path.join(tmpDir, "missing-home"); const env = makeEnv(tmpDir, { + graphicalSudo: true, bins: { getent: "echo 'picklab-lab:x:999:999::/var/empty:/bin/false'", sudo: "exit 0", @@ -824,9 +936,13 @@ describe("picklab setup lab-user", () => { expect(report.plan).toEqual([]); }); - it("runs each provisioning step through sudo and persists the config", async () => { + // Linux-only (see rationale above). + it.skipIf(process.platform !== "linux")( + "runs each provisioning step through sudo and persists the config", + async () => { const sudoLog = path.join(tmpDir, "sudo.log"); const env = makeEnv(tmpDir, { + graphicalSudo: true, bins: { sudo: `printf '%s\\n' "$*" >> ${sudoLog}\nexit 0` }, }); const result = await runCli( @@ -840,10 +956,10 @@ describe("picklab setup lab-user", () => { .trim() .split("\n"); expect(lines).toEqual([ - "-n useradd -r -M -s /usr/sbin/nologin picklab-lab", - "-n mkdir -p /var/lib/picklab/lab-home", - "-n chown picklab-lab:picklab-lab /var/lib/picklab/lab-home", - "-n chmod 750 /var/lib/picklab/lab-home", + "-A useradd -r -M -s /usr/sbin/nologin picklab-lab", + "-A mkdir -p /var/lib/picklab/lab-home", + "-A chown picklab-lab:picklab-lab /var/lib/picklab/lab-home", + "-A chmod 750 /var/lib/picklab/lab-home", ]); const globalConfig = JSON.parse( fs.readFileSync(path.join(env.PICKLAB_HOME!, "config.json"), "utf8"), @@ -854,9 +970,13 @@ describe("picklab setup lab-user", () => { }); }); - it("returns redacted partial results when a privileged step fails", async () => { + // Linux-only (see rationale above). + it.skipIf(process.platform !== "linux")( + "returns redacted partial results when a privileged step fails", + async () => { const sudoLog = path.join(tmpDir, "sudo.log"); const env = makeEnv(tmpDir, { + graphicalSudo: true, bins: { sudo: `printf '%s\\n' "$*" >> ${sudoLog}\n` + diff --git a/packages/cli/test/executor.test.ts b/packages/cli/test/executor.test.ts index 149f0a3..3f34152 100644 --- a/packages/cli/test/executor.test.ts +++ b/packages/cli/test/executor.test.ts @@ -2,14 +2,22 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import type { AskpassCapability } from "../src/provision/askpass.js"; import { classifyPlan, + createLocalExecutionAdapter, executeProvisioning, + PrivilegedCommandDeniedError, type ProvisioningExecutionAdapter, type ProvisioningSection, } from "../src/provision/executor.js"; import type { ProvisioningPlan, ProvisioningStep } from "../src/provision/plan.js"; +const AVAILABLE_ASKPASS: AskpassCapability = { + state: "available", + helper: "/usr/bin/ksshaskpass", +}; + let tmpDir: string; beforeEach(() => { @@ -333,7 +341,7 @@ describe("executeProvisioning", () => { }, ], { - privilege: { sudoPath: "/usr/bin/sudo", nonInteractive: true }, + privilege: { sudoPath: "/usr/bin/sudo", askpass: AVAILABLE_ASKPASS }, }, ); expect(result.status).toBe("declined"); @@ -342,7 +350,7 @@ describe("executeProvisioning", () => { command: { cmd: "/usr/bin/sudo", args: [ - "-n", + "-A", "useradd", "-r", "-M", @@ -350,6 +358,7 @@ describe("executeProvisioning", () => { "/usr/sbin/nologin", "picklab-lab", ], + env: { SUDO_ASKPASS: "/usr/bin/ksshaskpass" }, }, }); expect(raw).toMatchObject({ command: { cmd: "useradd" } }); @@ -394,7 +403,7 @@ describe("executeProvisioning", () => { }, }, ], - { privilege: { sudoPath: null, nonInteractive: true } }, + { privilege: { sudoPath: null } }, ); expect(result.status).toBe("failed"); expect(result.error).toBe("same missing sudo message"); @@ -430,7 +439,7 @@ describe("executeProvisioning", () => { }, { kind: "blocked", reason: "later error" }, ], - { privilege: { sudoPath: null, nonInteractive: true } }, + { privilege: { sudoPath: null } }, ); expect(result.plan.steps.map((step) => step.id)).toEqual(["mk"]); @@ -535,3 +544,192 @@ describe("executeProvisioning", () => { ).toEqual({ profile: "generic" }); }); }); + +// pickforge/picklab#27 — "Shared graphical sudo (askpass) security contract +// — locked v1". These tests cover the contract's verification list for the +// PickLab side: available/missing/headless preflight, cancellation, env +// propagation, arg-array safety, and redaction. +describe("privileged execution via graphical sudo (askpass)", () => { + const sudoPath = "/usr/bin/sudo"; + + it("materializes sudo -A with the resolved helper, adding only SUDO_ASKPASS to the env", () => { + const adapter = createLocalExecutionAdapter({ + privilege: { sudoPath, askpass: AVAILABLE_ASKPASS }, + }); + const step = command("root", true, ["-r", "-M", "picklab-lab"]); + if (step.kind !== "command") throw new Error("expected a command step"); + step.command.env = { ANDROID_HOME: "/sdk" }; + + const materialized = adapter.materialize(step); + if (materialized.kind !== "command") { + throw new Error("expected a command step"); + } + expect(materialized.command).toEqual({ + cmd: sudoPath, + args: ["-A", process.execPath, "-r", "-M", "picklab-lab"], + env: { ANDROID_HOME: "/sdk", SUDO_ASKPASS: "/usr/bin/ksshaskpass" }, + }); + // The raw step (and its env object) is untouched by materialization. + expect(step.command.env).toEqual({ ANDROID_HOME: "/sdk" }); + }); + + it("adds SUDO_ASKPASS as the only env key when the raw step declares no env", () => { + const adapter = createLocalExecutionAdapter({ + privilege: { sudoPath, askpass: AVAILABLE_ASKPASS }, + }); + const materialized = adapter.materialize(command("root", true)); + if (materialized.kind !== "command") { + throw new Error("expected a command step"); + } + expect(materialized.command.env).toEqual({ + SUDO_ASKPASS: "/usr/bin/ksshaskpass", + }); + }); + + it("keeps a hostile argv element as one array entry through materialization (never a shell string)", () => { + const adapter = createLocalExecutionAdapter({ + privilege: { sudoPath, askpass: AVAILABLE_ASKPASS }, + }); + const hostile = "$(touch /tmp/pwn) `evil`; rm -rf / | cat"; + const materialized = adapter.materialize(command("root", true, [hostile])); + if (materialized.kind !== "command") { + throw new Error("expected a command step"); + } + expect(materialized.command.args).toEqual([ + "-A", + process.execPath, + hostile, + ]); + expect(materialized.command.args).toHaveLength(3); + }); + + it.each([ + ["headless" as const, /graphical session/i], + ["no-helper" as const, /SUDO_ASKPASS helper/i], + ["unsupported-platform" as const, /only supported on Linux/i], + ])( + "fails preflight, before any mutation, when askpass capability is %s", + async (state, expected) => { + const target = path.join(tmpDir, "should-not-exist"); + const result = await executeProvisioning( + [ + { + kind: "plan", + plan: { + steps: [ + { + id: "mk", + title: "mk", + kind: "mkdir", + privileged: false, + dir: target, + }, + ], + }, + }, + { + kind: "plan", + plan: { steps: [command("root", true)] }, + consent: { decide: async () => ({ kind: "approved" }) }, + }, + ], + { privilege: { sudoPath, askpass: { state } } }, + ); + expect(result.status).toBe("failed"); + expect(result.error).toMatch(expected); + expect(result.error).toContain("Run it yourself in a terminal: sudo"); + expect(result.plan.steps.map((step) => step.id)).toEqual(["mk"]); + expect(result.results).toEqual([]); + expect(fs.existsSync(target)).toBe(false); + }, + ); + + it("fails preflight when privilege.askpass is omitted entirely (fail-closed default)", async () => { + const result = await executeProvisioning( + [approved({ steps: [command("root", true)] })], + { privilege: { sudoPath } }, + ); + expect(result.status).toBe("failed"); + expect(result.error).toMatch(/graphical session/i); + }); + + it( + "surfaces sudo cancellation/denial as a distinct 'cancelled' status, " + + "with no retry and an actionable manual fallback", + async () => { + const fakeSudo = path.join(tmpDir, "fake-sudo.cjs"); + const record = path.join(tmpDir, "fake-sudo-invocations.log"); + fs.writeFileSync( + fakeSudo, + "#!/usr/bin/env node\n" + + `require("fs").appendFileSync(${JSON.stringify(record)}, "invoked\\n");\n` + + 'process.stderr.write("sudo: a password is required\\n");\n' + + "process.exit(1);\n", + ); + fs.chmodSync(fakeSudo, 0o755); + + const result = await executeProvisioning( + [ + approved({ + steps: [command("root", true, ["-r", "-M", "picklab-lab"])], + }), + ], + { privilege: { sudoPath: fakeSudo, askpass: AVAILABLE_ASKPASS } }, + ); + + expect(result.status).toBe("cancelled"); + expect(result.ok).toBe(false); + expect(result.error).toContain("sudo denied or cancelled"); + expect(result.error).toContain("run it yourself in a terminal: sudo"); + expect(result.results).toEqual([ + { + id: "root", + ok: false, + detail: expect.stringContaining("cancelled"), + }, + ]); + // No automatic retry loop: the stand-in sudo ran exactly once. + expect(fs.readFileSync(record, "utf8").trim().split("\n")).toEqual([ + "invoked", + ]); + }, + ); + + it("redacts credential-shaped text out of a sudo denial message before it reaches the result", async () => { + const fakeSudo = path.join(tmpDir, "fake-sudo-secret.cjs"); + fs.writeFileSync( + fakeSudo, + "#!/usr/bin/env node\n" + + 'process.stderr.write("sudo: a password is required (token=planted-secret)\\n");\n' + + "process.exit(1);\n", + ); + fs.chmodSync(fakeSudo, 0o755); + + const result = await executeProvisioning( + [approved({ steps: [command("root", true)] })], + { privilege: { sudoPath: fakeSudo, askpass: AVAILABLE_ASKPASS } }, + ); + + expect(result.status).toBe("cancelled"); + expect(JSON.stringify(result)).not.toContain("planted-secret"); + expect(result.error).toContain("[REDACTED]"); + }); + + it("lets a custom adapter raise the same distinct cancelled state via the exported error class", async () => { + const result = await executeProvisioning( + [approved({ steps: [command("root", true)] })], + { + adapter: { + materialize: (step) => step, + execute: async () => {}, + executePrivileged: async () => { + throw new PrivilegedCommandDeniedError( + "sudo denied or cancelled this privileged command: test double", + ); + }, + }, + }, + ); + expect(result.status).toBe("cancelled"); + }); +}); From b6e3a87d48ec01967a6f2a28485a3c4681820024 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elberte=20Pl=C3=ADnio?= Date: Thu, 23 Jul 2026 02:34:59 -0300 Subject: [PATCH 2/2] fix(cli): expose executor status in json reports, fix lab-user skip prefix Add status (mirrors ExecuteProvisioningResult.status) to SetupLabUserReport, InitReport, and the doctor --fix report, so --json consumers can tell a sudo-cancelled run apart from declined/failed without string-matching. Fix prepareSections dropping a section's "lab-user: "/"avd: " skip-reason context for an askpass-unavailable failure (it only ever applied to the older sudo-missing case). Sections can now supply an optional privilegeUnavailable.context prefix that askpass-unavailable reasons honor too, restoring parity with sibling skip reasons in doctor --fix. --- packages/cli/src/commands/doctor.ts | 7 +++ packages/cli/src/commands/init.ts | 8 ++++ packages/cli/src/commands/setup-lab-user.ts | 8 ++++ packages/cli/src/provision/executor.ts | 25 +++++++++- packages/cli/test/cli.test.ts | 52 +++++++++++++++++++++ packages/cli/test/executor.test.ts | 43 +++++++++++++++++ 6 files changed, 141 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/commands/doctor.ts b/packages/cli/src/commands/doctor.ts index 5c4e2a7..c9bfcb5 100644 --- a/packages/cli/src/commands/doctor.ts +++ b/packages/cli/src/commands/doctor.ts @@ -8,6 +8,7 @@ import { import { collectSnapshot, type DetectionSnapshot } from "../provision/detect.js"; import { executeProvisioning, + type ExecuteProvisioningResult, type ProvisioningSection, type StepResult, } from "../provision/executor.js"; @@ -30,6 +31,10 @@ export interface DoctorCliOptions { export interface DoctorFixReport { dryRun: boolean; + /** Mirrors `ExecuteProvisioningResult.status` so `--json` consumers can + * distinguish a declined/cancelled/sudo-cancelled fix run from a generic + * failure without string-matching `steps`/errors. */ + status: ExecuteProvisioningResult["status"]; steps: ProvisioningStep[]; skipped: string[]; results: StepResult[]; @@ -116,6 +121,7 @@ async function buildFixPlan( reason: `lab-user: ${labUserPrivilegeUnavailableMessage( snapshot.labUser.name, )}`, + context: "lab-user: ", }, consent: { onDenied: "skip", @@ -176,6 +182,7 @@ export async function runDoctor( ); report.fix = { dryRun: opts.dryRun === true, + status: execution.status, steps: execution.plan.steps, skipped: execution.skipped, results: execution.results, diff --git a/packages/cli/src/commands/init.ts b/packages/cli/src/commands/init.ts index 345ba57..ab152ef 100644 --- a/packages/cli/src/commands/init.ts +++ b/packages/cli/src/commands/init.ts @@ -10,6 +10,7 @@ import { import { collectSnapshot, type DetectionSnapshot } from "../provision/detect.js"; import { executeProvisioning, + type ExecuteProvisioningResult, type ProvisioningSection, type StepResult, } from "../provision/executor.js"; @@ -34,6 +35,11 @@ export interface InitCliOptions { export interface InitReport { ok: boolean; + /** Mirrors `ExecuteProvisioningResult.status` so `--json` consumers can + * distinguish a declined/cancelled/sudo-cancelled provisioning run from a + * generic failure without string-matching `errors`. `"failed"` before an + * execution ever runs. */ + status: ExecuteProvisioningResult["status"]; profile: PicklabProfile; projectDir: string; dryRun: boolean; @@ -183,6 +189,7 @@ export async function runInit( const report: InitReport = { ok: false, + status: "failed", profile, projectDir, dryRun: opts.dryRun === true, @@ -210,6 +217,7 @@ export async function runInit( report.plan = execution.plan.steps; report.results = execution.results; report.ok = execution.ok; + report.status = execution.status; if (!execution.ok) { const errors = execution.errors.length > 0 ? execution.errors : ["provisioning failed"]; diff --git a/packages/cli/src/commands/setup-lab-user.ts b/packages/cli/src/commands/setup-lab-user.ts index e87fc99..44cd130 100644 --- a/packages/cli/src/commands/setup-lab-user.ts +++ b/packages/cli/src/commands/setup-lab-user.ts @@ -3,6 +3,7 @@ import { resolveAskpassCapability } from "../provision/askpass.js"; import { collectSnapshot } from "../provision/detect.js"; import { executeProvisioning, + type ExecuteProvisioningResult, type StepResult, } from "../provision/executor.js"; import type { ProvisioningStep } from "../provision/plan.js"; @@ -22,6 +23,11 @@ export interface SetupLabUserCliOptions { export interface SetupLabUserReport { ok: boolean; + /** Mirrors `ExecuteProvisioningResult.status` so `--json` consumers can + * distinguish a declined/cancelled/sudo-cancelled provisioning run from a + * generic failure without string-matching `errors`. `"failed"` before an + * execution ever runs (e.g. an invalid lab user name/home). */ + status: ExecuteProvisioningResult["status"]; name: string; home: string; userExists: boolean; @@ -52,6 +58,7 @@ export async function runSetupLabUser( }); const report: SetupLabUserReport = { ok: false, + status: "failed", name: snapshot.labUser.name, home: snapshot.labUser.home, userExists: snapshot.labUser.exists, @@ -121,6 +128,7 @@ export async function runSetupLabUser( report.plan = execution.plan.steps; report.results = execution.results; report.ok = execution.ok; + report.status = execution.status; if (!execution.ok) { report.errors.push(execution.error ?? "provisioning failed"); } diff --git a/packages/cli/src/provision/executor.ts b/packages/cli/src/provision/executor.ts index 20a1870..382d6f9 100644 --- a/packages/cli/src/provision/executor.ts +++ b/packages/cli/src/provision/executor.ts @@ -59,7 +59,19 @@ export interface ProvisioningPlanSection { }; privilegeUnavailable?: { action?: "error" | "skip"; + /** Used verbatim only when sudo itself is missing from PATH — the + * failure mode this field predates the askpass contract for. Ignored + * for an askpass-unavailable failure (see `context` below), since a + * static reason authored for "sudo not found" would otherwise paper + * over the real, state-correct askpass message. */ reason: string; + /** Prefix (e.g. `"lab-user: "`) prepended, verbatim, to an + * askpass-unavailable message (headless/no-helper/unsupported-platform). + * `reason` doesn't apply to that failure mode (see above), so this is a + * section's only way to keep the same "