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..c9bfcb5 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, @@ -7,6 +8,7 @@ import { import { collectSnapshot, type DetectionSnapshot } from "../provision/detect.js"; import { executeProvisioning, + type ExecuteProvisioningResult, type ProvisioningSection, type StepResult, } from "../provision/executor.js"; @@ -29,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[]; @@ -115,6 +121,7 @@ async function buildFixPlan( reason: `lab-user: ${labUserPrivilegeUnavailableMessage( snapshot.labUser.name, )}`, + context: "lab-user: ", }, consent: { onDenied: "skip", @@ -169,12 +176,13 @@ export async function runDoctor( log, privilege: { sudoPath: snapshot.sudo, - nonInteractive: process.stdin.isTTY !== true, + askpass: resolveAskpassCapability(env), }, }, ); 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 6b603ff..ab152ef 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, @@ -9,6 +10,7 @@ import { import { collectSnapshot, type DetectionSnapshot } from "../provision/detect.js"; import { executeProvisioning, + type ExecuteProvisioningResult, type ProvisioningSection, type StepResult, } from "../provision/executor.js"; @@ -33,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; @@ -182,6 +189,7 @@ export async function runInit( const report: InitReport = { ok: false, + status: "failed", profile, projectDir, dryRun: opts.dryRun === true, @@ -202,13 +210,14 @@ export async function runInit( log, privilege: { sudoPath: snapshot.sudo, - nonInteractive: process.stdin.isTTY !== true, + askpass: resolveAskpassCapability(env), }, }, ); 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 b1d590f..44cd130 100644 --- a/packages/cli/src/commands/setup-lab-user.ts +++ b/packages/cli/src/commands/setup-lab-user.ts @@ -1,7 +1,9 @@ import type { EnvLike } from "@pickforge/picklab-core"; +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"; @@ -21,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; @@ -51,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, @@ -108,7 +116,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) { @@ -120,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/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..382d6f9 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" @@ -41,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 "