diff --git a/docs/guides/native-candidate.md b/docs/guides/native-candidate.md index daae39b9d..cca0bfe49 100644 --- a/docs/guides/native-candidate.md +++ b/docs/guides/native-candidate.md @@ -1293,7 +1293,11 @@ built-in default such as `hack.local`, they stop with a usage message that names activation shows the exact scoped claim and requires interactive confirmation and a native administrator prompt. Run it from a terminal; the bounded privileged steps preserve that terminal's sudo authorization and report the underlying -command failure if a step fails. Before changing files, the candidate checks that +command failure if a step fails. If you approve but the administrator prompt +does not complete (it times out after 120 seconds, is interrupted, rejects the +password, or Hack is not the terminal's foreground job), activation fails during +authorization with that cause, says nothing changed, and suggests running +`sudo -v` in the same terminal first. Only answering no is reported as denied. Before changing files, the candidate checks that the root-owned Homebrew system launchd job matches the inspected dnsmasq command and configuration. It restarts that verified job directly through launchctl, so activation does not need Homebrew metadata downloads as root. Missing, changed or @@ -1333,6 +1337,16 @@ trust repair does not set up DNS or a browser's Local Network permission. Both authorization and installation retain the caller's controlling terminal, so terminal-scoped sudo authorization remains valid during the bounded repair. +Doctor's progress spinner stops before any repair asks a question. It stays off +through the native administrator prompt and the privileged steps that follow, +then resumes, so sudo's `Password:` prompt (or the Touch ID sheet) stays +visible. Doctor runs repairs one at a time. The administrator prompt (`sudo -v`) +runs in the terminal's foreground process group before any `sudo -n` step. If it +does not complete, the CA repair reports an error that names the cause and +suggests running `sudo -v` in the same terminal first. sudo itself prints +`a password is required` when its first password read is interrupted or times +out. + `hack.local` stays the default dev-host root. An older Docker-era install routes it to Docker Caddy with `address=/.hack.local/172.30.0.2` in Homebrew's `dnsmasq.conf`, which the native loopback relay cannot serve. On macOS, native @@ -1361,7 +1375,12 @@ Each unmet condition is a closed refusal reason: `relay-not-verified`, `dnsmasq-config-missing`, `unreadable-config`, `uninspected-config-source`, `no-managed-line`, `unmanaged-line`, `foreign-target`, `foreign-resolver`, `old-target-live`, `pending-receipt`, `invalid-receipt`, `non-interactive`, -`declined`, `busy`, `restart-unavailable` or `inputs-changed`. Refusal details +`declined`, `authorization-failed`, `busy`, `restart-unavailable` or +`inputs-changed`. `declined` means you answered no to the confirmation. +`authorization-failed` means you approved, but the native administrator prompt +did not complete: it timed out after 120 seconds, was interrupted, rejected the +password, or Hack was not the terminal's foreground job. Run `sudo -v` in the +same terminal, then rerun `hack doctor --fix` (or the rollback command). Refusal details name files and lines by number and never repeat file contents. The repair needs interactive confirmation and the native macOS administrator prompt; without a terminal it refuses and names `hack doctor --fix` to run in one. Only one repair or rollback runs at a time; a second one refuses diff --git a/src/commands/doctor.ts b/src/commands/doctor.ts index 12812853f..51a5e22ad 100644 --- a/src/commands/doctor.ts +++ b/src/commands/doctor.ts @@ -114,6 +114,7 @@ import { } from "../lib/mutagen.ts"; import { inspectNativeCaTrust, + type NativeCaDoctorDependencies, repairNativeCaTrust, } from "../lib/native-ca-doctor.ts"; import { @@ -161,6 +162,11 @@ import { import { readProjectsRegistry } from "../lib/projects-registry.ts"; import { readRuntimeProjects } from "../lib/runtime-projects.ts"; import { exec, findExecutableInPath, run } from "../lib/shell.ts"; +import { + type PausableProgress, + pausableSpinner, + withTerminalHandoff, +} from "../lib/terminal-handoff.ts"; import { resolveSessionsMuxMode } from "../mux/mux-config.ts"; import { renderGlobalCaddyCompose, @@ -1050,22 +1056,11 @@ async function handleNativeDoctor(opts: { message: "System-keychain repair requires macOS.", }; } - const repaired = await repairNativeCaTrust({ + return await repairNativeCaTrustCheck({ + progress, runtime: opts.runtime, hostname, }); - const messages = { - installed: - "Verified live native root installed in the System keychain.", - "already-trusted": "Verified live native root was already trusted.", - declined: - "Trust repair requires an interactive confirmation and native administrator prompt.", - } as const; - return { - name: "native ca repair", - status: repaired === "declined" ? "warn" : "ok", - message: messages[repaired], - }; }) ); } @@ -1188,6 +1183,46 @@ async function handleNativeDoctor(opts: { return hasError ? 1 : 0; } +/** + * The `native ca repair` check body. The spinner is released at the trust + * confirmation and stays off through the administrator prompt and the sudo -n + * install, so nothing erases the password prompt. + */ +export async function repairNativeCaTrustCheck(opts: { + readonly progress: DoctorProgress; + readonly runtime: NativeRuntimeSelection; + readonly hostname: string | undefined; + readonly dependencies?: Partial; +}): Promise { + const confirm = opts.dependencies?.confirm ?? confirmSafe; + const repaired = await withTerminalHandoff( + opts.progress, + async (claim) => + await repairNativeCaTrust({ + runtime: opts.runtime, + hostname: opts.hostname, + dependencies: { + ...opts.dependencies, + confirm: async (prompt) => { + claim(); + return await confirm(prompt); + }, + }, + }) + ); + const messages = { + installed: "Verified live native root installed in the System keychain.", + "already-trusted": "Verified live native root was already trusted.", + declined: + "Trust repair requires an interactive confirmation and native administrator prompt.", + } as const; + return { + name: "native ca repair", + status: repaired === "declined" ? "warn" : "ok", + message: messages[repaired], + }; +} + /** * Report, and with --fix authorize and apply, the native hack.local route * repair. Refusals are warnings with a closed reason; only an unexpected @@ -1217,7 +1252,23 @@ export async function checkNativeHackLocalRoute(opts: { if (opts.fix) { results.push( await runCheck(opts.progress, HACK_LOCAL_REPAIR_CHECK, async () => { - const repaired = await repairHackLocalRoute(deps); + // Release the terminal before the first prompt and keep it released + // through the administrator prompt and every later sudo -n step. + const repaired = await withTerminalHandoff( + opts.progress, + async (claim) => + await repairHackLocalRoute({ + ...deps, + authorize: async (message) => { + claim(); + return await deps.authorize(message); + }, + confirm: async (message) => { + claim(); + return await deps.confirm(message); + }, + }) + ); if (repaired.outcome === "repaired") { return { name: HACK_LOCAL_REPAIR_CHECK, @@ -1341,10 +1392,7 @@ export function assertDoctorOptionCompatibility(opts: { } } -type DoctorProgress = { - start(message?: string): void; - stop(message?: string): void; -}; +type DoctorProgress = PausableProgress; /** * Spinner wrapper that stays silent on non-TTY stdout (piped/automation) so @@ -1359,13 +1407,15 @@ function createDoctorProgress(opts: { process.stdout.isTTY === true && (process.env.NO_COLOR ?? "").trim().length === 0; if (interactive) { - return spinner(); + return pausableSpinner(spinner()); } return { start: (_message?: string) => { // Quiet in non-TTY/JSON mode. }, + pause: () => undefined, + resume: () => undefined, stop: (message?: string) => { if (!opts.json && message) { process.stdout.write(`${message}\n`); diff --git a/src/commands/global.ts b/src/commands/global.ts index d7e249d8e..26fd82073 100644 --- a/src/commands/global.ts +++ b/src/commands/global.ts @@ -57,6 +57,11 @@ import { getLaunchdServiceStatus } from "../daemon/launchd.ts"; import { resolveDaemonPaths } from "../daemon/paths.ts"; import { isProcessRunning } from "../daemon/process.ts"; import { readDaemonStatus } from "../daemon/status.ts"; +import { + ADMINISTRATOR_PROMPT_HINT, + describeAdministratorAuthorizationFailure, + requestAdministratorAuthorization, +} from "../lib/admin-authorization.ts"; import { resolveGlobalConfigPath, resolveGlobalHackDir, @@ -543,13 +548,7 @@ async function handleGlobalDnsActivate({ if (!accepted) { return false; } - return ( - (await run(["sudo", "-v"], { - stdin: "inherit", - timeoutMs: 120_000, - forwardSignals: true, - })) === 0 - ); + return await requireNativeAdministratorAuthorization(); }; const deps = createNativeDnsHostDependencies({ receiptPath: initial.receiptPath, @@ -668,13 +667,7 @@ async function handleGlobalDnsDeactivate({ if (!accepted) { return false; } - return ( - (await run(["sudo", "-v"], { - stdin: "inherit", - timeoutMs: 120_000, - forwardSignals: true, - })) === 0 - ); + return await requireNativeAdministratorAuthorization(); }; const deps = createNativeDnsHostDependencies({ receiptPath: initial.receiptPath, @@ -2345,6 +2338,21 @@ async function runMacPrivilegedCommand(opts: { }); } +/** + * Native administrator prompt after an approved DNS change. A failed prompt + * throws, so it is reported as an authorization failure and never as the + * user's decline. + */ +async function requireNativeAdministratorAuthorization(): Promise { + const result = await requestAdministratorAuthorization(); + if (result.status === "failed") { + throw new Error( + `macOS administrator authorization did not complete (${describeAdministratorAuthorizationFailure(result)}); nothing changed. ${ADMINISTRATOR_PROMPT_HINT}` + ); + } + return true; +} + function isInteractiveTerminal(): boolean { return process.stdin.isTTY === true; } diff --git a/src/lib/admin-authorization.ts b/src/lib/admin-authorization.ts new file mode 100644 index 000000000..d11d502ba --- /dev/null +++ b/src/lib/admin-authorization.ts @@ -0,0 +1,116 @@ +import { run } from "./shell.ts"; +import { openTerminalControl } from "./tty-process-group.ts"; + +/** + * Native macOS administrator authorization (`sudo -v`) shared by native + * repairs and DNS activation. + * + * Contract: + * - The caller has already obtained the human confirmation and released the + * terminal. No spinner or prompt may draw while sudo shows its password + * prompt; a redrawing spinner erases the prompt before the user sees it. + * - Hack must own the terminal's foreground process group. `run` then hands the + * foreground to the sudo child's own group and restores it afterwards. A + * background invocation is refused before sudo runs. + * - Later privileged steps use `sudo -n` with the ticket this call caches, so + * no `sudo -n` step may run before this call succeeds. + * - Any non-zero exit is an authorization failure, never a decline: the user + * already approved the change. sudo reports an interrupted or timed-out first + * password read as "a password is required". + */ +export const ADMINISTRATOR_PROMPT_TIMEOUT_MS = 120_000; + +/** Remediation shown with every authorization failure. */ +export const ADMINISTRATOR_PROMPT_HINT = + 'Run "sudo -v" in this terminal first, then retry.'; + +export type AdministratorAuthorizationFailure = + /** Hack is not the terminal's foreground job, so sudo cannot read a password. */ + | "not-foreground" + | "timed-out" + | "interrupted" + /** sudo exited non-zero, for example after three wrong passwords. */ + | "rejected"; + +export type AdministratorAuthorization = + | { readonly status: "authorized" } + | { + readonly status: "failed"; + readonly failure: AdministratorAuthorizationFailure; + /** The sudo exit code; null when sudo never ran. */ + readonly exitCode: number | null; + }; + +export interface AdministratorAuthorizationDependencies { + readonly runCommand: typeof run; + /** True when this process group owns the controlling terminal's foreground. */ + readonly ownsTerminalForeground: () => boolean; +} + +/** + * Whether this process may hand the terminal to a prompting child. An + * unavailable terminal-control library is unknown, not a refusal: sudo then + * reports its own failure. + */ +export function ownsTerminalForeground(): boolean { + let terminal: ReturnType; + try { + terminal = openTerminalControl(); + } catch { + return true; + } + try { + const foreground = terminal.foreground(); + return foreground > 0 && foreground === terminal.group(); + } finally { + terminal.close(); + } +} + +const DEFAULTS: AdministratorAuthorizationDependencies = { + runCommand: run, + ownsTerminalForeground, +}; + +/** Show the native administrator prompt once and classify its outcome. */ +export async function requestAdministratorAuthorization( + opts: { + readonly dependencies?: Partial; + } = {} +): Promise { + const deps = { ...DEFAULTS, ...opts.dependencies }; + if (!deps.ownsTerminalForeground()) { + return { status: "failed", failure: "not-foreground", exitCode: null }; + } + const exitCode = await deps.runCommand(["sudo", "-v"], { + stdin: "inherit", + timeoutMs: ADMINISTRATOR_PROMPT_TIMEOUT_MS, + forwardSignals: true, + }); + if (exitCode === 0) { + return { status: "authorized" }; + } + let failure: AdministratorAuthorizationFailure = "rejected"; + if (exitCode === 124) { + failure = "timed-out"; + } else if (exitCode === 130 || exitCode === 143) { + failure = "interrupted"; + } + return { status: "failed", failure, exitCode }; +} + +/** Control-free phrase naming why authorization did not complete. */ +export function describeAdministratorAuthorizationFailure( + result: Extract +): string { + switch (result.failure) { + case "not-foreground": + return "Hack is not the terminal's foreground job, so the macOS administrator prompt cannot read a password"; + case "timed-out": + return `the macOS administrator prompt timed out after ${ADMINISTRATOR_PROMPT_TIMEOUT_MS / 1000} seconds`; + case "interrupted": + return "the macOS administrator prompt was interrupted"; + default: + return `sudo -v did not accept the administrator password (exit ${result.exitCode ?? "unknown"})`; + } +} diff --git a/src/lib/native-ca-doctor.ts b/src/lib/native-ca-doctor.ts index b6c873b31..f9f17f48d 100644 --- a/src/lib/native-ca-doctor.ts +++ b/src/lib/native-ca-doctor.ts @@ -8,6 +8,12 @@ import { verifyNativeHttpsHostname, } from "../backends/native-project-https.ts"; import type { NativeRuntimeSelection } from "../backends/native-runtime-client.ts"; +import { + ADMINISTRATOR_PROMPT_HINT, + describeAdministratorAuthorizationFailure, + ownsTerminalForeground, + requestAdministratorAuthorization, +} from "./admin-authorization.ts"; import { canPrompt, confirmSafe } from "./interactivity.ts"; import { checkMacCaTrust } from "./mac-ca-trust.ts"; import { run } from "./shell.ts"; @@ -23,6 +29,7 @@ export interface NativeCaDoctorDependencies { readonly promptAllowed: typeof canPrompt; readonly confirm: typeof confirmSafe; readonly runCommand: typeof run; + readonly ownsTerminalForeground: () => boolean; } const DEFAULTS: NativeCaDoctorDependencies = { @@ -33,6 +40,7 @@ const DEFAULTS: NativeCaDoctorDependencies = { promptAllowed: canPrompt, confirm: confirmSafe, runCommand: run, + ownsTerminalForeground, }; function sameOwner(a: NativeCaOwner, b: NativeCaOwner): boolean { @@ -191,13 +199,16 @@ export async function repairNativeCaTrust(opts: { } finally { await file.close(); } - const prompted = await deps.runCommand(["sudo", "-v"], { - stdin: "inherit", - timeoutMs: 120_000, - forwardSignals: true, + const prompted = await requestAdministratorAuthorization({ + dependencies: { + runCommand: deps.runCommand, + ownsTerminalForeground: deps.ownsTerminalForeground, + }, }); - if (prompted !== 0) { - throw new Error("Native administrator authorization was not completed."); + if (prompted.status === "failed") { + throw new Error( + `Native administrator authorization did not complete (${describeAdministratorAuthorizationFailure(prompted)}); nothing was installed. ${ADMINISTRATOR_PROMPT_HINT}` + ); } const afterPrompt = await deps.inspect({ runtime: opts.runtime }); if (!sameOwner(confirmed, afterPrompt)) { diff --git a/src/lib/native-hack-local-route-host.ts b/src/lib/native-hack-local-route-host.ts index 53e61c524..251a27832 100644 --- a/src/lib/native-hack-local-route-host.ts +++ b/src/lib/native-hack-local-route-host.ts @@ -14,6 +14,11 @@ import { basename, dirname, join, resolve } from "node:path"; import { inspectActiveNativeHttpsOwner } from "../backends/native-project-https.ts"; import type { NativeRuntimeSelection } from "../backends/native-runtime-client.ts"; import { DEFAULT_HOST_DNS_IP } from "../constants.ts"; +import { + type AdministratorAuthorization, + describeAdministratorAuthorizationFailure, + requestAdministratorAuthorization, +} from "./admin-authorization.ts"; import { canPrompt, confirmSafe } from "./interactivity.ts"; import { type NativeDnsCommandRunner, @@ -27,10 +32,11 @@ import { } from "./native-domain-dns-snapshot.ts"; import { HACK_LOCAL_DOMAIN, + type HackLocalRouteAuthorization, type HackLocalRouteDependencies, type HackLocalRouteReceipt, } from "./native-hack-local-route.ts"; -import { exec, run } from "./shell.ts"; +import { exec } from "./shell.ts"; const IDENTITY = /^\d+:\d+$/; const IP_ADDRESS_LINE = /^ip_address:\s*(\S+)\s*$/gm; @@ -423,6 +429,38 @@ async function verifyLiveHackLocalDns(): Promise { } } +/** + * Confirmation first, then the native administrator prompt. A "no" never + * reaches sudo; an approved change whose prompt fails is reported as + * `authorization-failed`, never as a decline. + */ +export function createHackLocalRouteAuthorizer(deps: { + readonly confirm: (message: string) => Promise; + readonly requestAdministrator: () => Promise; +}): (message: string) => Promise { + return async (message) => { + if (!(await deps.confirm(message))) { + return { outcome: "declined" }; + } + const result = await deps.requestAdministrator(); + if (result.status === "authorized") { + return { outcome: "approved" }; + } + return { + outcome: "authorization-failed", + detail: describeAdministratorAuthorizationFailure(result), + }; + }; +} + +async function confirmRouteChange(message: string): Promise { + return await confirmSafe({ + message, + initialValue: false, + nonInteractive: "decline", + }); +} + /** * Production wiring for the native macOS host: Homebrew dnsmasq, the * /etc/resolver directory, the verified native relay owner, the system @@ -458,29 +496,12 @@ export async function createNativeHackLocalRouteDependencies(opts: { }, probeTarget: async (target) => await probeTcp({ host: target, port: 443 }), canPrompt: () => canPrompt(), - async authorize(message) { - const accepted = await confirmSafe({ - message, - initialValue: false, - nonInteractive: "decline", - }); - if (!accepted) { - return false; - } - return ( - (await run(["sudo", "-v"], { - stdin: "inherit", - timeoutMs: 120_000, - forwardSignals: true, - })) === 0 - ); - }, - confirm: async (message) => - await confirmSafe({ - message, - initialValue: false, - nonInteractive: "decline", - }), + authorize: createHackLocalRouteAuthorizer({ + confirm: confirmRouteChange, + requestAdministrator: async () => + await requestAdministratorAuthorization(), + }), + confirm: confirmRouteChange, async prepareRestart() { const host = await inspectNativeDnsHost({ brewPrefix: prefix }); if (host.mainConfig.path !== dnsmasqPath) { diff --git a/src/lib/native-hack-local-route.ts b/src/lib/native-hack-local-route.ts index 73669a950..7c8891244 100644 --- a/src/lib/native-hack-local-route.ts +++ b/src/lib/native-hack-local-route.ts @@ -83,7 +83,13 @@ export type HackLocalRouteRefusal = | "foreign-resolver" | "old-target-live" | "non-interactive" + /** The user answered no to the reviewed confirmation. */ | "declined" + /** + * The user approved, but the macOS administrator prompt did not complete + * (timed out, interrupted, wrong password or no foreground terminal). + */ + | "authorization-failed" | "busy" | "restart-unavailable" | "inputs-changed" @@ -122,6 +128,20 @@ export interface HackLocalRoutePaths { readonly receiptPath: string; } +/** + * Outcome of the confirmation plus the native administrator prompt. Only an + * answer of no is `declined`; an approved change whose administrator prompt + * did not complete is `authorization-failed`. + */ +export type HackLocalRouteAuthorization = + | { readonly outcome: "approved" } + | { readonly outcome: "declined" } + | { + readonly outcome: "authorization-failed"; + /** Control-free phrase naming why the prompt did not complete. */ + readonly detail: string; + }; + /** Every host read and effect is supplied by the caller. */ export interface HackLocalRouteDependencies { readonly paths: HackLocalRoutePaths; @@ -150,8 +170,11 @@ export interface HackLocalRouteDependencies { /** Bounded probe: true when the old target still answers on 443. */ readonly probeTarget: (target: string) => Promise; readonly canPrompt: () => boolean; - /** Human confirmation plus the native administrator prompt. */ - readonly authorize: (message: string) => Promise; + /** + * Human confirmation plus the native administrator prompt. Runs before any + * privileged step, which relies on the ticket this prompt caches. + */ + readonly authorize: (message: string) => Promise; /** Human confirmation only, for a change limited to the private receipt. */ readonly confirm: (message: string) => Promise; /** @@ -904,6 +927,26 @@ function busy(): { }; } +/** Refusal for an approved change whose administrator prompt did not complete. */ +function authorizationFailed( + why: string, + command: string +): { + readonly outcome: "refused"; + readonly reason: "authorization-failed"; + readonly detail: string; + readonly command: string; +} { + return { + outcome: "refused", + reason: "authorization-failed", + detail: printable( + `The macOS administrator prompt did not complete (${why}); nothing changed. Run "sudo -v" in this terminal first, then rerun: ${command}` + ), + command, + }; +} + /** * Authorized, reversible repair. Rechecks the plan after the administrator * prompt, publishes a pending receipt before effects, and on failure restores @@ -963,18 +1006,21 @@ async function repairUnderLock( plan.oldTarget === DEFAULT_HOST_DNS_IP ? "" : `${plan.oldTarget}:443 did not answer; `; - const approved = await deps.authorize( + const authorization = await deps.authorize( printable( `Native hack.local repair: ${changes.join(" and ")}? ${probe}the native relay listens on 127.0.0.1:${plan.relayPort}. This restarts dnsmasq. Undo: ${HACK_LOCAL_ROLLBACK_COMMAND}` ) ); - if (!approved) { + if (authorization.outcome === "declined") { return { outcome: "refused", reason: "declined", - detail: "The hack.local repair was not authorized; nothing changed.", + detail: "The hack.local repair was declined; nothing changed.", }; } + if (authorization.outcome === "authorization-failed") { + return authorizationFailed(authorization.detail, HACK_LOCAL_REPAIR_COMMAND); + } let restart: () => Promise; try { restart = await deps.prepareRestart(); @@ -1164,7 +1210,7 @@ async function rollbackUnderLock(opts: { if (step.kind === "drift") { return await releaseReceipt({ ...opts, samePaths }); } - const approved = await deps.authorize( + const authorization = await deps.authorize( printable( `Restore "${receipt.oldLine}" in ${receipt.dnsmasqPath}${ receipt.resolverAction === "create" @@ -1173,13 +1219,19 @@ async function rollbackUnderLock(opts: { }? This restarts dnsmasq.` ) ); - if (!approved) { + if (authorization.outcome === "declined") { return { outcome: "refused", reason: "declined", - detail: "The hack.local rollback was not authorized; nothing changed.", + detail: "The hack.local rollback was declined; nothing changed.", }; } + if (authorization.outcome === "authorization-failed") { + return authorizationFailed( + authorization.detail, + HACK_LOCAL_ROLLBACK_COMMAND + ); + } // Restoring the file needs no service; a missing or ambiguous dnsmasq only // means the restored line loads on its next start. let restart: (() => Promise) | null = null; diff --git a/src/lib/terminal-handoff.ts b/src/lib/terminal-handoff.ts new file mode 100644 index 000000000..d7b9ad9b7 --- /dev/null +++ b/src/lib/terminal-handoff.ts @@ -0,0 +1,87 @@ +/** + * Terminal handoff between a redrawing progress indicator and interactive + * steps (confirmation prompts and the native administrator prompt). + * + * A clack spinner redraws its line every frame by moving to column 1 and + * erasing to the end of the screen, and it holds stdin in a keypress reader. + * Left running, it erases a sudo password prompt within one frame, so the user + * never sees it. Interactive steps therefore claim the terminal first; the + * indicator stays paused until the whole step settles (including its `sudo -n` + * follow-ups) and resumes once. + */ + +/** Progress indicator that can release and later retake the terminal. */ +export interface PausableProgress { + start(message?: string): void; + stop(message?: string): void; + /** Erase the indicator and stop drawing; a no-op when not drawing. */ + pause(): void; + /** Draw again with the last message, only if `pause` stopped it. */ + resume(): void; +} + +/** The subset of a clack spinner the adapter drives. */ +export interface SpinnerLike { + start(message?: string): void; + stop(message?: string): void; + clear(): void; +} + +/** Make a spinner pausable without losing its current message. */ +export function pausableSpinner(indicator: SpinnerLike): PausableProgress { + let message: string | undefined; + let state: "idle" | "drawing" | "paused" = "idle"; + return { + start(next) { + message = next; + state = "drawing"; + indicator.start(next); + }, + stop(final) { + if (state === "paused") { + indicator.start(message); + } + state = "idle"; + indicator.stop(final); + }, + pause() { + if (state !== "drawing") { + return; + } + state = "paused"; + indicator.clear(); + }, + resume() { + if (state !== "paused") { + return; + } + state = "drawing"; + indicator.start(message); + }, + }; +} + +/** + * Run a step that may prompt. `claim` pauses the indicator before the first + * prompt; a step that never claims never pauses. After a claim the indicator + * resumes exactly once, when the step settles or throws. + */ +export async function withTerminalHandoff( + progress: Pick, + step: (claim: () => void) => Promise +): Promise { + let claimed = false; + const claim = () => { + if (!claimed) { + claimed = true; + progress.pause(); + } + }; + try { + return await step(claim); + } finally { + if (claimed) { + progress.resume(); + } + } +} diff --git a/tests/native-admin-authorization.test.ts b/tests/native-admin-authorization.test.ts new file mode 100644 index 000000000..9c9f36358 --- /dev/null +++ b/tests/native-admin-authorization.test.ts @@ -0,0 +1,205 @@ +import { expect, test } from "bun:test"; +import { + type AdministratorAuthorization, + type AdministratorAuthorizationFailure, + ownsTerminalForeground, + requestAdministratorAuthorization, +} from "../src/lib/admin-authorization.ts"; +import { createHackLocalRouteAuthorizer } from "../src/lib/native-hack-local-route-host.ts"; +import type { RunOptions } from "../src/lib/shell.ts"; +import { + pausableSpinner, + withTerminalHandoff, +} from "../src/lib/terminal-handoff.ts"; +import { hasControllingTerminal } from "../src/lib/tty-process-group.ts"; + +/** Records sudo invocations; never runs a real command. */ +function fakeRunner(exitCode: number) { + const calls: { command: string[]; options: RunOptions | undefined }[] = []; + return { + calls, + runCommand: async (command: readonly string[], options?: RunOptions) => { + calls.push({ command: [...command], options }); + return exitCode; + }, + }; +} + +test("the administrator prompt runs sudo -v in the foreground terminal, never sudo -n", async () => { + const runner = fakeRunner(0); + expect( + await requestAdministratorAuthorization({ + dependencies: { + runCommand: runner.runCommand, + ownsTerminalForeground: () => true, + }, + }) + ).toEqual({ status: "authorized" }); + expect(runner.calls).toEqual([ + { + command: ["sudo", "-v"], + options: { stdin: "inherit", timeoutMs: 120_000, forwardSignals: true }, + }, + ]); +}); + +test("sudo failures are classified, never treated as a decline", async () => { + const cases: readonly [number, AdministratorAuthorizationFailure][] = [ + [124, "timed-out"], + [130, "interrupted"], + [143, "interrupted"], + [1, "rejected"], + ]; + for (const [exitCode, failure] of cases) { + const runner = fakeRunner(exitCode); + expect( + await requestAdministratorAuthorization({ + dependencies: { + runCommand: runner.runCommand, + ownsTerminalForeground: () => true, + }, + }) + ).toEqual({ status: "failed", failure, exitCode }); + } +}); + +test("a background run refuses before sudo can stop or read nothing", async () => { + const runner = fakeRunner(0); + expect( + await requestAdministratorAuthorization({ + dependencies: { + runCommand: runner.runCommand, + ownsTerminalForeground: () => false, + }, + }) + ).toEqual({ status: "failed", failure: "not-foreground", exitCode: null }); + expect(runner.calls).toEqual([]); +}); + +test.skipIf(process.stdin.isTTY === true || hasControllingTerminal())( + "without a terminal the process never owns a terminal foreground", + () => { + expect(ownsTerminalForeground()).toBe(false); + } +); + +test("route authorization confirms first and only then prompts for the administrator", async () => { + const order: string[] = []; + const outcome = (result: AdministratorAuthorization) => + createHackLocalRouteAuthorizer({ + confirm: async () => { + order.push("confirm"); + return true; + }, + requestAdministrator: async () => { + order.push("sudo -v"); + return result; + }, + }); + + expect(await outcome({ status: "authorized" })("Repair?")).toEqual({ + outcome: "approved", + }); + expect(order).toEqual(["confirm", "sudo -v"]); + + expect( + await outcome({ status: "failed", failure: "timed-out", exitCode: 124 })( + "Repair?" + ) + ).toEqual({ + outcome: "authorization-failed", + detail: "the macOS administrator prompt timed out after 120 seconds", + }); + expect( + await outcome({ status: "failed", failure: "rejected", exitCode: 1 })( + "Repair?" + ) + ).toEqual({ + outcome: "authorization-failed", + detail: "sudo -v did not accept the administrator password (exit 1)", + }); +}); + +test("a no at the confirmation is declined and never reaches sudo", async () => { + const order: string[] = []; + const authorize = createHackLocalRouteAuthorizer({ + confirm: async () => { + order.push("confirm"); + return false; + }, + requestAdministrator: async () => { + order.push("sudo -v"); + return { status: "authorized" }; + }, + }); + expect(await authorize("Repair?")).toEqual({ outcome: "declined" }); + expect(order).toEqual(["confirm"]); +}); + +function recordingSpinner() { + const events: string[] = []; + return { + events, + indicator: { + start: (message?: string) => events.push(`start:${message ?? ""}`), + stop: (message?: string) => events.push(`stop:${message ?? ""}`), + clear: () => events.push("clear"), + }, + }; +} + +test("the spinner is erased before a prompt and redrawn with its message after", async () => { + const spinner = recordingSpinner(); + const progress = pausableSpinner(spinner.indicator); + progress.start("Checking the active native HTTPS root..."); + const result = await withTerminalHandoff(progress, async (claim) => { + spinner.events.push("inspect"); + claim(); + spinner.events.push("prompt"); + claim(); + spinner.events.push("sudo -n"); + return "done"; + }); + expect(result).toBe("done"); + progress.stop("Native doctor checks complete"); + expect(spinner.events).toEqual([ + "start:Checking the active native HTTPS root...", + "inspect", + "clear", + "prompt", + "sudo -n", + "start:Checking the active native HTTPS root...", + "stop:Native doctor checks complete", + ]); +}); + +test("a step that never prompts keeps the spinner drawing", async () => { + const spinner = recordingSpinner(); + const progress = pausableSpinner(spinner.indicator); + progress.start("Checking"); + await withTerminalHandoff(progress, async () => { + spinner.events.push("inspect"); + }); + expect(spinner.events).toEqual(["start:Checking", "inspect"]); +}); + +test("a failing step still returns the terminal to the spinner", async () => { + const spinner = recordingSpinner(); + const progress = pausableSpinner(spinner.indicator); + progress.start("Checking"); + await expect( + withTerminalHandoff(progress, async (claim) => { + claim(); + throw new Error("prompt failed"); + }) + ).rejects.toThrow("prompt failed"); + expect(spinner.events).toEqual(["start:Checking", "clear", "start:Checking"]); +}); + +test("pausing an idle spinner draws nothing", () => { + const spinner = recordingSpinner(); + const progress = pausableSpinner(spinner.indicator); + progress.pause(); + progress.resume(); + expect(spinner.events).toEqual([]); +}); diff --git a/tests/native-ca-doctor.test.ts b/tests/native-ca-doctor.test.ts index a74b50033..0baf44fe2 100644 --- a/tests/native-ca-doctor.test.ts +++ b/tests/native-ca-doctor.test.ts @@ -90,6 +90,10 @@ async function fixture() { } return 0; }, + ownsTerminalForeground: () => { + calls.push("foreground"); + return true; + }, }; return { home, @@ -275,6 +279,96 @@ test("rotation after the sudo prompt prevents keychain installation", async () = } }); +test("a failed administrator prompt names the cause and never installs", async () => { + const f = await fixture(); + try { + await expect( + repairNativeCaTrust({ + runtime: f.runtime, + hostname: "app.hack.gy", + dependencies: { + ...f.dependencies, + runCommand: async (cmd) => { + f.calls.push(cmd[1] === "-v" ? "sudo-prompt" : "install"); + return cmd[1] === "-v" ? 124 : 0; + }, + }, + }) + ).rejects.toThrow( + 'Native administrator authorization did not complete (the macOS administrator prompt timed out after 120 seconds); nothing was installed. Run "sudo -v" in this terminal first, then retry.' + ); + const confirmed = f.calls.indexOf("confirm"); + expect(confirmed).toBeGreaterThanOrEqual(0); + expect(f.calls.slice(confirmed)).toEqual([ + "confirm", + "inspect", + "foreground", + "sudo-prompt", + ]); + } finally { + await f.cleanup(); + } +}); + +test("doctor releases the spinner before the trust confirmation and the sudo prompt", async () => { + const { repairNativeCaTrustCheck } = await import( + "../src/commands/doctor.ts" + ); + const f = await fixture(); + const progress = { + start: () => f.calls.push("spinner-start"), + stop: () => f.calls.push("spinner-stop"), + pause: () => f.calls.push("spinner-pause"), + resume: () => f.calls.push("spinner-resume"), + }; + try { + expect( + await repairNativeCaTrustCheck({ + progress, + runtime: f.runtime, + hostname: "app.hack.gy", + dependencies: f.dependencies, + }) + ).toMatchObject({ name: "native ca repair", status: "ok" }); + const paused = f.calls.indexOf("spinner-pause"); + expect(paused).toBeGreaterThanOrEqual(0); + expect(paused).toBeLessThan(f.calls.indexOf("confirm")); + expect(f.calls.indexOf("confirm")).toBeLessThan( + f.calls.indexOf("sudo-prompt") + ); + expect(f.calls.indexOf("sudo-prompt")).toBeLessThan( + f.calls.indexOf("install") + ); + expect(f.calls.at(-1)).toBe("spinner-resume"); + expect(f.calls.filter((call) => call.startsWith("spinner-"))).toEqual([ + "spinner-pause", + "spinner-resume", + ]); + } finally { + await f.cleanup(); + } +}); + +test("a background doctor run never shows the administrator prompt", async () => { + const f = await fixture(); + try { + await expect( + repairNativeCaTrust({ + runtime: f.runtime, + hostname: "app.hack.gy", + dependencies: { + ...f.dependencies, + ownsTerminalForeground: () => false, + }, + }) + ).rejects.toThrow("not the terminal's foreground job"); + expect(f.calls).not.toContain("sudo-prompt"); + expect(f.calls).not.toContain("install"); + } finally { + await f.cleanup(); + } +}); + test("confirmed native repair stages the exact root and checks trust afterward", async () => { const f = await fixture(); try { diff --git a/tests/native-ca-rotation-wire.test.ts b/tests/native-ca-rotation-wire.test.ts index d739849c9..0c5957ea8 100644 --- a/tests/native-ca-rotation-wire.test.ts +++ b/tests/native-ca-rotation-wire.test.ts @@ -217,6 +217,9 @@ while True: "Fixture must not invoke sudo or the real System keychain" ); }, + ownsTerminalForeground: () => { + throw new Error("Fixture must not inspect the real terminal"); + }, }; const result = await inspectNativeCaTrust({ runtime, diff --git a/tests/native-hack-local-route.test.ts b/tests/native-hack-local-route.test.ts index 634510591..84bdf7f9a 100644 --- a/tests/native-hack-local-route.test.ts +++ b/tests/native-hack-local-route.test.ts @@ -25,6 +25,7 @@ import { HACK_LOCAL_REPAIR_COMMAND, HACK_LOCAL_RESOLVER_CONTENT, HACK_LOCAL_ROLLBACK_COMMAND, + type HackLocalRouteAuthorization, type HackLocalRouteDependencies, type HackLocalRoutePlan, type HackLocalRouteReceipt, @@ -63,6 +64,9 @@ afterEach(async () => { } }); +const APPROVED: HackLocalRouteAuthorization = { outcome: "approved" }; +const DECLINED: HackLocalRouteAuthorization = { outcome: "declined" }; + interface Calls { readonly authorize: string[]; readonly confirm: string[]; @@ -179,7 +183,7 @@ async function fixture( canPrompt: () => opts.prompt ?? true, authorize: async (message) => { calls.authorize.push(message); - return opts.approve ?? true; + return opts.approve === false ? DECLINED : APPROVED; }, confirm: async (message) => { calls.confirm.push(message); @@ -466,13 +470,161 @@ test("non-interactive and declined runs refuse with an exact command", async () } }); +test("a failed administrator prompt is authorization-failed, never declined", async () => { + const f = await fixture(); + const steps: string[] = []; + const deps: HackLocalRouteDependencies = { + ...f.deps, + authorize: async () => ({ + outcome: "authorization-failed", + detail: "the macOS administrator prompt timed out after 120 seconds", + }), + prepareRestart: async () => { + steps.push("prepare-restart"); + return async () => { + steps.push("restart"); + }; + }, + createResolver: async () => { + steps.push("sudo -n resolver"); + return "1:1"; + }, + flushDnsCache: async () => { + steps.push("sudo -n flush"); + }, + }; + const result = await repairHackLocalRoute(deps); + expect(result).toEqual({ + outcome: "refused", + reason: "authorization-failed", + detail: expect.stringContaining('Run "sudo -v" in this terminal first'), + command: HACK_LOCAL_REPAIR_COMMAND, + }); + if (result.outcome !== "refused") { + throw new Error("expected a refusal"); + } + expect(result.detail).toContain("timed out after 120 seconds"); + expect(result.detail).toContain("nothing changed"); + expect(steps).toEqual([]); + expect(await readFile(f.dnsmasqPath, "utf8")).toBe(ORIGINAL); + await expect(stat(f.deps.paths.receiptPath)).rejects.toThrow(); +}); + +test("the administrator prompt precedes every privileged sudo -n step", async () => { + const f = await fixture(); + const steps: string[] = []; + const deps: HackLocalRouteDependencies = { + ...f.deps, + authorize: async (message) => { + steps.push("authorize"); + return await f.deps.authorize(message); + }, + prepareRestart: async () => { + steps.push("prepare-restart"); + return async () => { + steps.push("sudo -n kickstart"); + }; + }, + createResolver: async (content) => { + steps.push("sudo -n resolver"); + return await f.deps.createResolver(content); + }, + flushDnsCache: async () => { + steps.push("sudo -n flush"); + }, + }; + expect((await repairHackLocalRoute(deps)).outcome).toBe("repaired"); + expect(steps).toEqual([ + "authorize", + "prepare-restart", + "sudo -n resolver", + "sudo -n kickstart", + "sudo -n flush", + ]); +}); + +test("a failed administrator prompt during rollback changes nothing", async () => { + const f = await fixture(); + expect((await repairHackLocalRoute(f.deps)).outcome).toBe("repaired"); + const receiptBefore = await readFile(f.deps.paths.receiptPath, "utf8"); + const result = await rollbackHackLocalRoute({ + ...f.deps, + authorize: async () => ({ + outcome: "authorization-failed", + detail: "the macOS administrator prompt was interrupted", + }), + }); + expect(result).toMatchObject({ + outcome: "refused", + reason: "authorization-failed", + command: HACK_LOCAL_ROLLBACK_COMMAND, + }); + expect(await readFile(f.dnsmasqPath, "utf8")).toBe(REWRITTEN); + expect(await readFile(f.deps.paths.receiptPath, "utf8")).toBe(receiptBefore); +}); + +test("doctor releases the spinner before the repair prompt and resumes after", async () => { + const { checkNativeHackLocalRoute } = await import( + "../src/commands/doctor.ts" + ); + const events: string[] = []; + const progress = { + start: () => events.push("start"), + stop: () => events.push("stop"), + pause: () => events.push("pause"), + resume: () => events.push("resume"), + }; + const failed = await fixture(); + const results = await checkNativeHackLocalRoute({ + progress, + fix: true, + dependencies: async () => ({ + ...failed.deps, + authorize: async () => { + events.push("authorize"); + return { + outcome: "authorization-failed", + detail: "the macOS administrator prompt timed out after 120 seconds", + }; + }, + }), + }); + expect(events).toEqual(["pause", "authorize", "resume"]); + expect(results[0]).toMatchObject({ + name: "native hack.local repair", + status: "warn", + }); + expect(results[0]?.message).toContain( + "Repair refused (authorization-failed)" + ); + expect(results[0]?.message).toContain('Run "sudo -v" in this terminal first'); + expect(results[0]?.message).not.toContain("declined"); + + events.length = 0; + const quiet = await fixture({ prompt: false }); + const quietResults = await checkNativeHackLocalRoute({ + progress, + fix: true, + dependencies: async () => ({ + ...quiet.deps, + authorize: async () => { + events.push("authorize"); + return APPROVED; + }, + }), + }); + // Non-interactive runs neither prompt nor release the terminal. + expect(events).toEqual([]); + expect(quietResults[0]?.message).toContain("non-interactive"); +}); + test("a change during authorization refuses before any effect", async () => { const f = await fixture(); const deps: HackLocalRouteDependencies = { ...f.deps, authorize: async () => { await writeFile(f.dnsmasqPath, `${ORIGINAL}# edited\n`); - return true; + return APPROVED; }, }; expect(await repairHackLocalRoute(deps)).toMatchObject({ @@ -694,7 +846,12 @@ test("doctor --fix applies the repair and JSON reports the route state", async ( const { checkNativeHackLocalRoute, buildDoctorJsonData } = await import( "../src/commands/doctor.ts" ); - const progress = { start: () => undefined, stop: () => undefined }; + const progress = { + start: () => undefined, + stop: () => undefined, + pause: () => undefined, + resume: () => undefined, + }; const reportOnly = await fixture(); const reported = await checkNativeHackLocalRoute({ progress, @@ -828,7 +985,7 @@ test("rollback without a terminal refuses with the exact command and changes not const declined = await rollbackHackLocalRoute({ ...f.deps, - authorize: async () => false, + authorize: async () => DECLINED, }); expect(declined).toMatchObject({ outcome: "refused", reason: "declined" }); expect(await readFile(f.dnsmasqPath, "utf8")).toBe(REWRITTEN); @@ -845,7 +1002,7 @@ test("rollback rechecks the file and receipt after authorization", async () => { ...fileChanged.deps, authorize: async () => { await writeFile(fileChanged.dnsmasqPath, edited); - return true; + return APPROVED; }, }) ).toMatchObject({ outcome: "refused", reason: "inputs-changed" }); @@ -861,7 +1018,7 @@ test("rollback rechecks the file and receipt after authorization", async () => { ...receiptChanged.deps, authorize: async () => { await editReceipt(receiptChanged.deps, {}); - return true; + return APPROVED; }, }) ).toMatchObject({ outcome: "refused", reason: "inputs-changed" });