diff --git a/ci/test-file-size-budget.json b/ci/test-file-size-budget.json index 23cc3326ed..dbe56635a5 100644 --- a/ci/test-file-size-budget.json +++ b/ci/test-file-size-budget.json @@ -9,7 +9,6 @@ "test/install-preflight.test.ts": 3921, "test/nemoclaw-start.test.ts": 4819, "test/onboard-messaging.test.ts": 2049, - "test/onboard-selection.test.ts": 4769, - "test/policies.test.ts": 1530 + "test/onboard-selection.test.ts": 4769 } } diff --git a/src/lib/actions/sandbox/policy-channel-policy.test.ts b/src/lib/actions/sandbox/policy-channel-policy.test.ts index ac22c9e49d..4c42a58c3e 100644 --- a/src/lib/actions/sandbox/policy-channel-policy.test.ts +++ b/src/lib/actions/sandbox/policy-channel-policy.test.ts @@ -176,6 +176,28 @@ describe("addSandboxPolicy", () => { expect(applyPresetMock).not.toHaveBeenCalled(); }); + it("exits non-zero when the add picker reaches stdin EOF (#7418)", async () => { + selectFromListMock.mockRejectedValueOnce( + Object.assign(new Error("Prompt closed before input"), { code: "EOF" }), + ); + + await expect(captureExit(() => addSandboxPolicy("test-sandbox"))).resolves.toBe(1); + + // Names the real condition rather than reporting non-interactive mode, + // which is not what happened here. + expect(printedText()).toContain("No input available on stdin"); + expect(applyPresetMock).not.toHaveBeenCalled(); + }); + + it("propagates a non-EOF picker failure instead of exiting (#7418)", async () => { + const failure = Object.assign(new Error("stdin read failed"), { code: "EIO" }); + selectFromListMock.mockRejectedValueOnce(failure); + + await expect(addSandboxPolicy("test-sandbox")).rejects.toBe(failure); + + expect(applyPresetMock).not.toHaveBeenCalled(); + }); + it("filters Hermes-only presets from the OpenClaw picker", async () => { arrangeSandbox("openclaw"); @@ -363,4 +385,15 @@ describe("removeSandboxPolicy", () => { expect(printedText()).toContain("Non-interactive mode requires a preset name."); expect(removePresetMock).not.toHaveBeenCalled(); }); + + it("exits non-zero when the remove picker reaches stdin EOF (#7418)", async () => { + selectForRemovalMock.mockRejectedValueOnce( + Object.assign(new Error("Prompt closed before input"), { code: "EOF" }), + ); + + await expect(captureExit(() => removeSandboxPolicy("test-sandbox"))).resolves.toBe(1); + + expect(printedText()).toContain("No input available on stdin"); + expect(removePresetMock).not.toHaveBeenCalled(); + }); }); diff --git a/src/lib/actions/sandbox/policy-channel.ts b/src/lib/actions/sandbox/policy-channel.ts index 254e6dfda0..7b680ffde8 100644 --- a/src/lib/actions/sandbox/policy-channel.ts +++ b/src/lib/actions/sandbox/policy-channel.ts @@ -63,6 +63,52 @@ import { executeSandboxCommand, executeSandboxExecCommand } from "./process-reco const isNonInteractive = isNonInteractiveEnv; +/** + * Report that `NEMOCLAW_NON_INTERACTIVE=1` leaves no interactive picker, and + * exit non-zero. + */ +function exitPresetNameRequired(usage: string): never { + console.error(" Non-interactive mode requires a preset name."); + console.error(` Usage: ${usage}`); + process.exit(1); +} + +/** + * Report that the picker prompt reached stdin EOF, and exit non-zero. + * + * Separate from `exitPresetNameRequired` because the conditions differ. That + * one means the operator set `NEMOCLAW_NON_INTERACTIVE=1`. This one means + * stdin closed while that variable was unset, so naming the variable would + * misdirect whoever reads the boot-unit log. + */ +function exitPromptStdinClosed(usage: string): never { + console.error(" No input available on stdin, so the preset picker cannot prompt."); + console.error(` Usage: ${usage}`); + process.exit(1); +} + +/** + * Await an interactive preset picker and convert a prompt EOF into exit 1. + * + * A boot unit that pipes or closes stdin without setting + * `NEMOCLAW_NON_INTERACTIVE=1` reaches the picker, and the prompt then hits + * EOF. Before #7418 the picker promise never settled, so the command exited 0 + * having changed nothing. Automation could not distinguish an applied preset + * from a no-op. Any other failure propagates unchanged. + */ +async function pickPresetOrExit( + pick: () => Promise, + usage: string, +): Promise { + try { + return await pick(); + } catch (error) { + const code = (error as NodeJS.ErrnoException | null)?.code; + if (code !== "EOF") throw error; + exitPromptStdinClosed(usage); + } +} + type ChannelMutationOptions = { channel?: string; dryRun?: boolean; @@ -217,12 +263,11 @@ async function addSandboxPolicyUnlocked( } answer = preset.name; } else { + const usage = `${CLI_NAME} policy-add [--yes] [--dry-run]`; if (isNonInteractive()) { - console.error(" Non-interactive mode requires a preset name."); - console.error(` Usage: ${CLI_NAME} policy-add [--yes] [--dry-run]`); - process.exit(1); + exitPresetNameRequired(usage); } - answer = await policies.selectFromList(allPresets, { applied }); + answer = await pickPresetOrExit(() => policies.selectFromList(allPresets, { applied }), usage); } if (!answer) return; @@ -1632,12 +1677,14 @@ async function removeSandboxPolicyUnlocked( } answer = preset.name; } else { + const usage = `${CLI_NAME} policy-remove [--yes] [--dry-run]`; if (isNonInteractive()) { - console.error(" Non-interactive mode requires a preset name."); - console.error(` Usage: ${CLI_NAME} policy-remove [--yes] [--dry-run]`); - process.exit(1); + exitPresetNameRequired(usage); } - answer = await policies.selectForRemoval(allPresets, { applied }); + answer = await pickPresetOrExit( + () => policies.selectForRemoval(allPresets, { applied }), + usage, + ); } if (!answer) return; diff --git a/src/lib/policy/index.ts b/src/lib/policy/index.ts index 10ebbb60d1..0951e78d7a 100644 --- a/src/lib/policy/index.ts +++ b/src/lib/policy/index.ts @@ -838,58 +838,88 @@ function removePreset( } /** - * Interactive preset picker for the `policy-remove` command. Prompts on - * stderr and resolves to the chosen preset name, or `null` if the user - * cancels or enters an invalid selection. + * Ask one preset-picker question on stderr and resolve to the raw answer. + * + * Rejects with `code: "EOF"` when readline closes before the question is + * answered. A boot unit runs `nemoclaw policy-add < /dev/null`, and + * the `question` callback then never fires. Without this handler the picker + * promise never settles and the command exits 0 having applied nothing + * (#7418). + * + * `finish` marks the prompt done before closing readline, because + * `rl.close()` itself emits `close`. Only a close that arrives before an + * answer rejects. + * + * Rejects with `code: "SIGINT"` when the operator presses Ctrl-C, and + * re-raises the signal so the process dies by SIGINT. Readline emits `close` + * for an interrupt as well as for EOF, so without a SIGINT listener an + * interrupt would be reported as a closed stdin. + * + * This matches the `prompt()` contract in `credentials/store.ts` (#5976). */ -function selectForRemoval( - items: PresetInfo[], - { applied = [] }: SelectionOptions = {}, -): Promise { - return new Promise((resolve) => { - const appliedItems = items.filter((item) => applied.includes(item.name)); - if (appliedItems.length === 0) { - process.stderr.write("\n No presets are currently applied.\n\n"); - resolve(null); - return; - } - process.stderr.write("\n Applied presets:\n"); - appliedItems.forEach((item, i) => { - const description = item.description ? ` — ${item.description}` : ""; - process.stderr.write(` ${i + 1}) ${item.name}${description}\n`); - }); - process.stderr.write("\n"); - const question = " Choose preset to remove: "; +function askPreset(question: string): Promise { + return new Promise((resolve, reject) => { // Re-attach stdin to the event loop — unref() on exit is sticky and // would otherwise leave a follow-up prompt waiting on a detached handle. if (typeof process.stdin.ref === "function") process.stdin.ref(); const rl = readline.createInterface({ input: process.stdin, output: process.stderr }); - rl.question(question, (answer: string) => { + let finished = false; + const finish = (settle: () => void) => { + if (finished) return; + finished = true; rl.close(); // pause+unref so the process exits naturally after the last prompt. // The matching ref() above keeps subsequent prompts working. if (typeof process.stdin.pause === "function") process.stdin.pause(); if (typeof process.stdin.unref === "function") process.stdin.unref(); - const trimmed = answer.trim(); - if (!trimmed) { - resolve(null); - return; - } - if (!/^\d+$/.test(trimmed)) { - process.stderr.write("\n Invalid preset number.\n"); - resolve(null); - return; - } - const num = Number(trimmed); - const item = appliedItems[num - 1]; - if (!item) { - process.stderr.write("\n Invalid preset number.\n"); - resolve(null); - return; - } - resolve(item.name); + settle(); + }; + // Runs before the `close` listener below, so an interrupt settles as + // SIGINT and the close that follows is ignored. + rl.on("SIGINT", () => { + finish(() => reject(Object.assign(new Error("Prompt interrupted"), { code: "SIGINT" }))); + process.kill(process.pid, "SIGINT"); }); + rl.on("close", () => + finish(() => reject(Object.assign(new Error("Prompt closed before input"), { code: "EOF" }))), + ); + rl.question(question, (answer: string) => finish(() => resolve(answer))); + }); +} + +/** + * Interactive preset picker for the `policy-remove` command. Prompts on + * stderr and resolves to the chosen preset name, or `null` if the user + * cancels or enters an invalid selection. Rejects with `code: "EOF"` when + * stdin closes before an answer (see `askPreset`). + */ +async function selectForRemoval( + items: PresetInfo[], + { applied = [] }: SelectionOptions = {}, +): Promise { + const appliedItems = items.filter((item) => applied.includes(item.name)); + if (appliedItems.length === 0) { + process.stderr.write("\n No presets are currently applied.\n\n"); + return null; + } + process.stderr.write("\n Applied presets:\n"); + appliedItems.forEach((item, i) => { + const description = item.description ? ` — ${item.description}` : ""; + process.stderr.write(` ${i + 1}) ${item.name}${description}\n`); }); + process.stderr.write("\n"); + const trimmed = (await askPreset(" Choose preset to remove: ")).trim(); + if (!trimmed) return null; + if (!/^\d+$/.test(trimmed)) { + process.stderr.write("\n Invalid preset number.\n"); + return null; + } + const item = appliedItems[Number(trimmed) - 1]; + if (!item) { + process.stderr.write("\n Invalid preset number.\n"); + return null; + } + return item.name; } /** @@ -1496,64 +1526,45 @@ function presetContentMatchesGateway(sandboxName: string, presetContent: string) /** * Interactive preset picker for the `policy-add` command. Prints the * presets on stderr (● applied, ○ not applied), prompts for a number, and - * resolves to the chosen preset name or `null` on cancel. + * resolves to the chosen preset name or `null` on cancel. Rejects with + * `code: "EOF"` when stdin closes before an answer (see `askPreset`). */ -function selectFromList( +async function selectFromList( items: PresetInfo[], { applied = [] }: SelectionOptions = {}, ): Promise { - return new Promise((resolve) => { - process.stderr.write("\n Available presets:\n"); - items.forEach((item, i) => { - const marker = applied.includes(item.name) ? "●" : "○"; - const description = item.description ? ` — ${item.description}` : ""; - process.stderr.write(` ${i + 1}) ${marker} ${item.name}${description}\n`); - }); - process.stderr.write("\n ● applied, ○ not applied\n\n"); - const defaultIdx = items.findIndex((item) => !applied.includes(item.name)); - const defaultNum = defaultIdx >= 0 ? defaultIdx + 1 : null; - const question = defaultNum ? ` Choose preset [${defaultNum}]: ` : " Choose preset: "; - // Re-attach stdin to the event loop — unref() on exit is sticky and - // would otherwise leave a follow-up prompt waiting on a detached handle. - if (typeof process.stdin.ref === "function") process.stdin.ref(); - const rl = readline.createInterface({ input: process.stdin, output: process.stderr }); - rl.question(question, (answer: string) => { - rl.close(); - // pause+unref so the process exits naturally after the last prompt. - // The matching ref() above keeps subsequent prompts working. - if (typeof process.stdin.pause === "function") process.stdin.pause(); - if (typeof process.stdin.unref === "function") process.stdin.unref(); - const trimmed = answer.trim(); - const effectiveInput = trimmed || (defaultNum ? String(defaultNum) : ""); - if (!effectiveInput) { - resolve(null); - return; - } - if (!/^\d+$/.test(effectiveInput)) { - process.stderr.write("\n Invalid preset number.\n"); - resolve(null); - return; - } - const num = Number(effectiveInput); - const item = items[num - 1]; - if (!item) { - process.stderr.write("\n Invalid preset number.\n"); - resolve(null); - return; - } - if (applied.includes(item.name)) { - // The picker has no live-policy context to classify drift; the named - // path (policy-add ) re-applies edited presets (#7323). - process.stderr.write(`\n Preset '${item.name}' is already applied.\n`); - process.stderr.write( - ` If its preset file changed, run '${CLI_NAME} policy-add ${item.name}' to re-apply it.\n`, - ); - resolve(null); - return; - } - resolve(item.name); - }); + process.stderr.write("\n Available presets:\n"); + items.forEach((item, i) => { + const marker = applied.includes(item.name) ? "●" : "○"; + const description = item.description ? ` — ${item.description}` : ""; + process.stderr.write(` ${i + 1}) ${marker} ${item.name}${description}\n`); }); + process.stderr.write("\n ● applied, ○ not applied\n\n"); + const defaultIdx = items.findIndex((item) => !applied.includes(item.name)); + const defaultNum = defaultIdx >= 0 ? defaultIdx + 1 : null; + const question = defaultNum ? ` Choose preset [${defaultNum}]: ` : " Choose preset: "; + const trimmed = (await askPreset(question)).trim(); + const effectiveInput = trimmed || (defaultNum ? String(defaultNum) : ""); + if (!effectiveInput) return null; + if (!/^\d+$/.test(effectiveInput)) { + process.stderr.write("\n Invalid preset number.\n"); + return null; + } + const item = items[Number(effectiveInput) - 1]; + if (!item) { + process.stderr.write("\n Invalid preset number.\n"); + return null; + } + if (applied.includes(item.name)) { + // The picker has no live-policy context to classify drift; the named + // path (policy-add ) re-applies edited presets (#7323). + process.stderr.write(`\n Preset '${item.name}' is already applied.\n`); + process.stderr.write( + ` If its preset file changed, run '${CLI_NAME} policy-add ${item.name}' to re-apply it.\n`, + ); + return null; + } + return item.name; } const PERMISSIVE_POLICY_PATH = path.join( diff --git a/test/package-contract/cli/policy-prompt-eof.test.ts b/test/package-contract/cli/policy-prompt-eof.test.ts new file mode 100644 index 0000000000..d767d05613 --- /dev/null +++ b/test/package-contract/cli/policy-prompt-eof.test.ts @@ -0,0 +1,92 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +/** + * Prompt-cancellation package contract for the policy preset pickers (#7418). + * + * A boot unit runs `nemoclaw policy-add` with no preset name and a + * closed stdin. That reaches the interactive picker, and the prompt hits EOF. + * The `question` callback never fired, the picker promise never settled, and + * the CLI exited 0 having applied nothing. Automation could not distinguish + * an applied preset from a no-op. + * + * These tests drive the compiled CLI (`dist/nemoclaw.js`) on real stdin at + * EOF, so readline decides when the prompt closes. Only the registry and + * preset lookups are stubbed, which replaces on-disk sandbox state. + */ + +import { spawnSync } from "node:child_process"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +import { describe, expect, it } from "vitest"; + +const REPO_ROOT = path.join(import.meta.dirname, "../../.."); +const CLI_PATH = JSON.stringify(path.join(REPO_ROOT, "dist", "nemoclaw.js")); +const POLICIES_PATH = JSON.stringify(path.join(REPO_ROOT, "dist", "lib", "policy", "index.js")); +const REGISTRY_PATH = JSON.stringify(path.join(REPO_ROOT, "dist", "lib", "state", "registry.js")); + +/** + * Run a policy command with no preset name against a closed stdin. `input: ""` + * gives the child an already-ended pipe, which is the EOF a boot unit + * produces. + */ +function runPolicyCommandAtStdinEof(command: "policy-add" | "policy-remove") { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-policy-prompt-eof-")); + const scriptPath = path.join(tmpDir, "policy-prompt-eof-check.js"); + const script = String.raw` +const registry = require(${REGISTRY_PATH}); +const policies = require(${POLICIES_PATH}); +policies.listPresets = () => [ + { file: "npm.yaml", name: "npm", description: "npm registry access" }, + { file: "pypi.yaml", name: "pypi", description: "PyPI access" }, +]; +policies.listCustomPresets = () => []; +policies.getAppliedPresets = () => ["npm"]; +registry.getSandbox = (name) => + name === "test-sandbox" ? { name, policies: ["npm"], customPolicies: [] } : null; +registry.listSandboxes = () => ({ sandboxes: [{ name: "test-sandbox" }] }); +process.argv = ["node", "nemoclaw.js", "test-sandbox", ${JSON.stringify(command)}]; +require(${CLI_PATH}); +`; + fs.writeFileSync(scriptPath, script); + try { + return spawnSync(process.execPath, [scriptPath], { + cwd: REPO_ROOT, + encoding: "utf-8", + input: "", + // Keep defensive hang protection around the real stdin boundary. Before + // #7418, the unresolved picker promise let Node exit 0 after stdin closed; + // the status assertion below catches that original regression. + timeout: 30_000, + killSignal: "SIGKILL", + env: { ...process.env, HOME: tmpDir, NEMOCLAW_NON_INTERACTIVE: undefined }, + }); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } +} + +describe("policy preset prompt cancellation", () => { + it.each([ + { command: "policy-add" as const, menu: "Available presets:" }, + { command: "policy-remove" as const, menu: "Applied presets:" }, + ])("$command exits non-zero when the picker prompt hits EOF (#7418)", ({ command, menu }) => { + const result = runPolicyCommandAtStdinEof(command); + + // The child exited on its own rather than being killed by the defensive + // timeout above. A hang would produce SIGKILL and a null status; the + // pre-#7418 regression exited 0 and is caught by the final assertion. + expect(result.error).toBeUndefined(); + expect(result.signal).toBeNull(); + // The picker was reached, so this is prompt EOF rather than the + // NEMOCLAW_NON_INTERACTIVE=1 guard exiting earlier. + expect(result.stderr).toContain(menu); + expect(result.stderr).toContain("No input available on stdin"); + expect(result.stderr).toContain(`${command} `); + expect(result.status).toBe(1); + // Above the child's 30s cap, so any hang fails on the assertions above + // rather than as a bare suite timeout. + }, 45_000); +}); diff --git a/test/policies.test.ts b/test/policies.test.ts index fea960c35d..665559b575 100644 --- a/test/policies.test.ts +++ b/test/policies.test.ts @@ -6,11 +6,9 @@ import fs from "node:fs"; import { createRequire } from "node:module"; import os from "node:os"; import path from "node:path"; -import type { Interface as ReadlineInterface } from "node:readline"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; const requireForTest = createRequire(import.meta.url); -const readline = requireForTest("node:readline") as typeof import("node:readline"); const YAML = requireForTest("yaml"); const REPO_ROOT = path.join(import.meta.dirname, ".."); const policies = requireForTest( @@ -22,77 +20,6 @@ const resolveOpenshellModule = requireForTest( const POLICIES_PATH = JSON.stringify(path.join(REPO_ROOT, "src", "lib", "policy", "index.ts")); const REGISTRY_PATH = JSON.stringify(path.join(REPO_ROOT, "src", "lib", "state", "registry.ts")); const SOURCE_NODE_ARGS = ["--import", "tsx"]; -const SELECT_FROM_LIST_ITEMS = [ - { name: "npm", description: "npm and Yarn registry access", file: "npm.yaml" }, - { name: "pypi", description: "Python Package Index (PyPI) access", file: "pypi.yaml" }, -]; -type AppliedOptions = { - applied?: string[]; -}; - -type SelectionFunction = "selectFromList" | "selectForRemoval"; - -async function runSelectionPrompt( - functionName: SelectionFunction, - input: string, - { applied = [] }: AppliedOptions = {}, -) { - const stderr: string[] = []; - const counts = { ref: 0, pause: 0, unref: 0 }; - const stdin = process.stdin as typeof process.stdin & { - ref: () => typeof process.stdin; - pause: () => typeof process.stdin; - unref: () => typeof process.stdin; - }; - const original = { - ref: stdin.ref, - pause: stdin.pause, - unref: stdin.unref, - }; - const stderrWrite = vi.spyOn(process.stderr, "write").mockImplementation(((chunk: unknown) => { - stderr.push(String(chunk)); - return true; - }) as typeof process.stderr.write); - const close = vi.fn(); - const createInterface = vi.spyOn(readline, "createInterface").mockImplementation((options) => { - expect(options).toEqual({ input: process.stdin, output: process.stderr }); - return { - question: (question: string, callback: (answer: string) => void) => { - process.stderr.write(question); - callback(input); - }, - close, - } as unknown as ReadlineInterface; - }); - stdin.ref = () => { - counts.ref += 1; - return process.stdin; - }; - stdin.pause = () => { - counts.pause += 1; - return process.stdin; - }; - stdin.unref = () => { - counts.unref += 1; - return process.stdin; - }; - - try { - const selected = await policies[functionName](SELECT_FROM_LIST_ITEMS, { applied }); - return { - selected, - stderr: stderr.join(""), - counts, - close, - }; - } finally { - stdin.ref = original.ref; - stdin.pause = original.pause; - stdin.unref = original.unref; - createInterface.mockRestore(); - stderrWrite.mockRestore(); - } -} function requirePresetContent(content: string | null): string { expect(content).toBeTruthy(); @@ -1165,61 +1092,6 @@ exit 1 }); }); - describe("selectFromList", () => { - it("returns preset name by number from stdin input", async () => { - const result = await runSelectionPrompt("selectFromList", "1\n"); - - expect(result.selected).toBe("npm"); - expect(result.stderr).toContain("Choose preset [1]:"); - }); - - it("uses the first preset as the default when input is empty", async () => { - const result = await runSelectionPrompt("selectFromList", "\n"); - - expect(result.stderr).toContain("Choose preset [1]:"); - expect(result.selected).toBe("npm"); - }); - - it("defaults to the first not-applied preset", async () => { - const result = await runSelectionPrompt("selectFromList", "\n", { applied: ["npm"] }); - - expect(result.stderr).toContain("Choose preset [2]:"); - expect(result.selected).toBe("pypi"); - }); - - it("rejects selecting an already-applied preset", async () => { - const result = await runSelectionPrompt("selectFromList", "1\n", { applied: ["npm"] }); - - expect(result.stderr).toMatch(/already applied\.[\s\S]*policy-add npm'/); - expect(result.selected).toBeNull(); - }); - - it("rejects out-of-range preset number", async () => { - const result = await runSelectionPrompt("selectFromList", "99\n"); - - expect(result.stderr).toContain("Invalid preset number."); - expect(result.selected).toBeNull(); - }); - - it("rejects non-numeric preset input", async () => { - const result = await runSelectionPrompt("selectFromList", "npm\n"); - - expect(result.stderr).toContain("Invalid preset number."); - expect(result.selected).toBeNull(); - }); - - it("prints numbered list with applied markers, legend, and default prompt", async () => { - const result = await runSelectionPrompt("selectFromList", "2\n", { applied: ["npm"] }); - - expect(result.stderr).toMatch(/Available presets:/); - expect(result.stderr).toMatch(/1\) ● npm — npm and Yarn registry access/); - expect(result.stderr).toMatch(/2\) ○ pypi — Python Package Index \(PyPI\) access/); - expect(result.stderr).toMatch(/● applied, ○ not applied/); - expect(result.stderr).toMatch(/Choose preset \[2\]:/); - expect(result.selected).toBe("pypi"); - }); - }); - describe("removePresetFromPolicy", () => { const pypiEntries = " pypi:\n" + @@ -1295,50 +1167,6 @@ exit 1 }); }); - describe("selectForRemoval", () => { - it("returns null when no presets are applied", async () => { - const result = await runSelectionPrompt("selectForRemoval", "1\n", { applied: [] }); - expect(result.stderr).toContain("No presets are currently applied"); - expect(result.selected).toBeNull(); - }); - - it("shows only applied presets and returns selected name", async () => { - const result = await runSelectionPrompt("selectForRemoval", "1\n", { applied: ["npm"] }); - expect(result.stderr).toContain("Applied presets:"); - expect(result.stderr).toContain("1) npm"); - expect(result.stderr).not.toContain("pypi"); - expect(result.selected).toBe("npm"); - }); - - it("returns null for empty input", async () => { - const result = await runSelectionPrompt("selectForRemoval", "\n", { applied: ["npm"] }); - expect(result.selected).toBeNull(); - }); - - it("rejects non-numeric input", async () => { - const result = await runSelectionPrompt("selectForRemoval", "npm\n", { - applied: ["npm"], - }); - expect(result.stderr).toContain("Invalid preset number"); - expect(result.selected).toBeNull(); - }); - - it("rejects out-of-range number", async () => { - const result = await runSelectionPrompt("selectForRemoval", "99\n", { applied: ["npm"] }); - expect(result.stderr).toContain("Invalid preset number"); - expect(result.selected).toBeNull(); - }); - - it("selects second preset when both are applied", async () => { - const result = await runSelectionPrompt("selectForRemoval", "2\n", { - applied: ["npm", "pypi"], - }); - expect(result.stderr).toContain("1) npm"); - expect(result.stderr).toContain("2) pypi"); - expect(result.selected).toBe("pypi"); - }); - }); - describe("loadPresetFromFile", () => { const tmpDirs: string[] = []; afterEach(() => { @@ -1507,24 +1335,4 @@ exit 1 } }); }); - - describe("interactive prompt cleanup", () => { - it("releases and re-refs stdin around policy-add preset prompts", async () => { - const result = await runSelectionPrompt("selectFromList", "1\n"); - expect(result.selected).toBe("npm"); - expect(result.counts.ref).toBeGreaterThanOrEqual(1); - expect(result.counts.pause).toBeGreaterThanOrEqual(1); - expect(result.counts.unref).toBeGreaterThanOrEqual(1); - expect(result.close).toHaveBeenCalledOnce(); - }); - - it("releases and re-refs stdin around policy-remove preset prompts", async () => { - const result = await runSelectionPrompt("selectForRemoval", "1\n", { applied: ["npm"] }); - expect(result.selected).toBe("npm"); - expect(result.counts.ref).toBeGreaterThanOrEqual(1); - expect(result.counts.pause).toBeGreaterThanOrEqual(1); - expect(result.counts.unref).toBeGreaterThanOrEqual(1); - expect(result.close).toHaveBeenCalledOnce(); - }); - }); }); diff --git a/test/policy-preset-picker.test.ts b/test/policy-preset-picker.test.ts new file mode 100644 index 0000000000..036e63d488 --- /dev/null +++ b/test/policy-preset-picker.test.ts @@ -0,0 +1,382 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +/** + * Interactive preset pickers for `policy-add` and `policy-remove`: selection + * parsing, stdin event-loop cleanup, and prompt-EOF cancellation (#7418). + */ + +import { createRequire } from "node:module"; +import path from "node:path"; +import type { Interface as ReadlineInterface } from "node:readline"; +import { describe, expect, it, vi } from "vitest"; + +const requireForTest = createRequire(import.meta.url); +const readline = requireForTest("node:readline") as typeof import("node:readline"); +const REPO_ROOT = path.join(import.meta.dirname, ".."); +const policies = requireForTest( + path.join(REPO_ROOT, "src", "lib", "policy", "index.ts"), +) as typeof import("../src/lib/policy"); + +const SELECT_FROM_LIST_ITEMS = [ + { name: "npm", description: "npm and Yarn registry access", file: "npm.yaml" }, + { name: "pypi", description: "Python Package Index (PyPI) access", file: "pypi.yaml" }, +]; +type AppliedOptions = { + applied?: string[]; +}; + +type SelectionFunction = "selectFromList" | "selectForRemoval"; + +async function runSelectionPrompt( + functionName: SelectionFunction, + input: string, + { applied = [] }: AppliedOptions = {}, +) { + const stderr: string[] = []; + const counts = { ref: 0, pause: 0, unref: 0 }; + const stdin = process.stdin as typeof process.stdin & { + ref: () => typeof process.stdin; + pause: () => typeof process.stdin; + unref: () => typeof process.stdin; + }; + const original = { + ref: stdin.ref, + pause: stdin.pause, + unref: stdin.unref, + }; + const stderrWrite = vi.spyOn(process.stderr, "write").mockImplementation(((chunk: unknown) => { + stderr.push(String(chunk)); + return true; + }) as typeof process.stderr.write); + // Readline emits `close` whenever `rl.close()` runs, including the + // `rl.close()` the picker performs after an answer. The fake emits it too, + // so a successful selection exercises the picker's reentrancy guard. Were + // that guard removed, the post-answer close would settle the promise a + // second time (#7418). + const closeListeners: Array<() => void> = []; + const close = vi.fn(() => closeListeners.forEach((listener) => listener())); + const createInterface = vi.spyOn(readline, "createInterface").mockImplementation((options) => { + expect(options).toEqual({ input: process.stdin, output: process.stderr }); + return { + question: (question: string, callback: (answer: string) => void) => { + process.stderr.write(question); + callback(input); + }, + on: (event: string, listener: () => void) => { + // No `if`: changed test files may not add one (codebase-growth-guardrails). + [listener].filter(() => event === "close").forEach((l) => closeListeners.push(l)); + }, + close, + } as unknown as ReadlineInterface; + }); + stdin.ref = () => { + counts.ref += 1; + return process.stdin; + }; + stdin.pause = () => { + counts.pause += 1; + return process.stdin; + }; + stdin.unref = () => { + counts.unref += 1; + return process.stdin; + }; + + try { + const selected = await policies[functionName](SELECT_FROM_LIST_ITEMS, { applied }); + return { + selected, + stderr: stderr.join(""), + counts, + close, + }; + } finally { + stdin.ref = original.ref; + stdin.pause = original.pause; + stdin.unref = original.unref; + createInterface.mockRestore(); + stderrWrite.mockRestore(); + } +} + +/** + * Drive a picker against a readline interface that reaches EOF. `question` + * writes the prompt but its callback never fires, and readline closes + * instead. A boot unit produces this by running `policy-add < /dev/null`. + */ +async function runSelectionPromptAtEof( + functionName: SelectionFunction, + { applied = [] }: AppliedOptions = {}, +) { + const stderr: string[] = []; + // Stub the same stdin methods `runSelectionPrompt` stubs. The picker calls + // ref/pause/unref on the real handle otherwise, which leaves this Vitest + // worker's stdin unreferenced and makes later tests order-dependent. + const stdin = process.stdin as typeof process.stdin & { + ref: () => typeof process.stdin; + pause: () => typeof process.stdin; + unref: () => typeof process.stdin; + }; + const original = { ref: stdin.ref, pause: stdin.pause, unref: stdin.unref }; + stdin.ref = () => process.stdin; + stdin.pause = () => process.stdin; + stdin.unref = () => process.stdin; + const stderrWrite = vi.spyOn(process.stderr, "write").mockImplementation(((chunk: unknown) => { + stderr.push(String(chunk)); + return true; + }) as typeof process.stderr.write); + const closeListeners: Array<() => void> = []; + const createInterface = vi.spyOn(readline, "createInterface").mockImplementation( + () => + ({ + question: (question: string) => { + process.stderr.write(question); + // Real readline emits `close` on EOF without answering. + queueMicrotask(() => closeListeners.forEach((listener) => listener())); + }, + on: (event: string, listener: () => void) => { + [listener].filter(() => event === "close").forEach((l) => closeListeners.push(l)); + }, + close: vi.fn(), + }) as unknown as ReadlineInterface, + ); + + try { + return await policies[functionName](SELECT_FROM_LIST_ITEMS, { applied }).then( + (selected) => ({ outcome: "resolved", selected, code: undefined, stderr: stderr.join("") }), + (error: NodeJS.ErrnoException) => ({ + outcome: "rejected", + selected: undefined, + code: error.code, + stderr: stderr.join(""), + }), + ); + } finally { + stdin.ref = original.ref; + stdin.pause = original.pause; + stdin.unref = original.unref; + createInterface.mockRestore(); + stderrWrite.mockRestore(); + } +} + +/** + * Drive a picker against a readline interface that receives an interrupt. + * Readline emits `SIGINT` and then `close`, so this proves an interrupt is + * reported as SIGINT rather than as a closed stdin (#7418). + */ +async function runSelectionPromptAtSigint( + functionName: SelectionFunction, + { applied = [] }: AppliedOptions = {}, +) { + const stdin = process.stdin as typeof process.stdin & { + ref: () => typeof process.stdin; + pause: () => typeof process.stdin; + unref: () => typeof process.stdin; + }; + const original = { ref: stdin.ref, pause: stdin.pause, unref: stdin.unref }; + stdin.ref = () => process.stdin; + stdin.pause = () => process.stdin; + stdin.unref = () => process.stdin; + const stderrWrite = vi + .spyOn(process.stderr, "write") + .mockImplementation((() => true) as typeof process.stderr.write); + // The picker re-raises SIGINT; capture it instead of killing the worker. + const kill = vi.spyOn(process, "kill").mockImplementation((() => true) as typeof process.kill); + const listeners = new Map void>(); + const createInterface = vi.spyOn(readline, "createInterface").mockImplementation( + () => + ({ + question: () => { + queueMicrotask(() => { + listeners.get("SIGINT")?.(); + listeners.get("close")?.(); + }); + }, + on: (event: string, listener: () => void) => { + listeners.set(event, listener); + }, + close: vi.fn(), + }) as unknown as ReadlineInterface, + ); + + try { + return await policies[functionName](SELECT_FROM_LIST_ITEMS, { applied }).then( + () => ({ + code: undefined as string | undefined, + reraised: kill.mock.calls.length, + signal: kill.mock.calls[0]?.[1], + }), + (error: NodeJS.ErrnoException) => ({ + code: error.code, + reraised: kill.mock.calls.length, + signal: kill.mock.calls[0]?.[1], + }), + ); + } finally { + stdin.ref = original.ref; + stdin.pause = original.pause; + stdin.unref = original.unref; + createInterface.mockRestore(); + stderrWrite.mockRestore(); + kill.mockRestore(); + } +} + +describe("policy preset pickers", () => { + describe("selectFromList", () => { + it("returns preset name by number from stdin input", async () => { + const result = await runSelectionPrompt("selectFromList", "1\n"); + + expect(result.selected).toBe("npm"); + expect(result.stderr).toContain("Choose preset [1]:"); + }); + + it("uses the first preset as the default when input is empty", async () => { + const result = await runSelectionPrompt("selectFromList", "\n"); + + expect(result.stderr).toContain("Choose preset [1]:"); + expect(result.selected).toBe("npm"); + }); + + it("defaults to the first not-applied preset", async () => { + const result = await runSelectionPrompt("selectFromList", "\n", { applied: ["npm"] }); + + expect(result.stderr).toContain("Choose preset [2]:"); + expect(result.selected).toBe("pypi"); + }); + + it("rejects selecting an already-applied preset", async () => { + const result = await runSelectionPrompt("selectFromList", "1\n", { applied: ["npm"] }); + + expect(result.stderr).toMatch(/already applied\.[\s\S]*policy-add npm'/); + expect(result.selected).toBeNull(); + }); + + it("rejects out-of-range preset number", async () => { + const result = await runSelectionPrompt("selectFromList", "99\n"); + + expect(result.stderr).toContain("Invalid preset number."); + expect(result.selected).toBeNull(); + }); + + it("rejects non-numeric preset input", async () => { + const result = await runSelectionPrompt("selectFromList", "npm\n"); + + expect(result.stderr).toContain("Invalid preset number."); + expect(result.selected).toBeNull(); + }); + + it("prints numbered list with applied markers, legend, and default prompt", async () => { + const result = await runSelectionPrompt("selectFromList", "2\n", { applied: ["npm"] }); + + expect(result.stderr).toMatch(/Available presets:/); + expect(result.stderr).toMatch(/1\) ● npm — npm and Yarn registry access/); + expect(result.stderr).toMatch(/2\) ○ pypi — Python Package Index \(PyPI\) access/); + expect(result.stderr).toMatch(/● applied, ○ not applied/); + expect(result.stderr).toMatch(/Choose preset \[2\]:/); + expect(result.selected).toBe("pypi"); + }); + + it("rejects with code EOF when stdin closes before an answer (#7418)", async () => { + const result = await runSelectionPromptAtEof("selectFromList"); + + expect(result.stderr).toContain("Choose preset [1]:"); + expect(result.outcome).toBe("rejected"); + expect(result.code).toBe("EOF"); + }, 3_000); + + it("reports an interrupt as SIGINT rather than closed stdin (#7418)", async () => { + const result = await runSelectionPromptAtSigint("selectFromList"); + + expect(result.code).toBe("SIGINT"); + expect(result.reraised).toBe(1); + // The signal itself, not just that kill ran: re-raising SIGTERM would + // otherwise satisfy this test. + expect(result.signal).toBe("SIGINT"); + }, 3_000); + }); + + describe("selectForRemoval", () => { + it("returns null when no presets are applied", async () => { + const result = await runSelectionPrompt("selectForRemoval", "1\n", { applied: [] }); + expect(result.stderr).toContain("No presets are currently applied"); + expect(result.selected).toBeNull(); + }); + + it("shows only applied presets and returns selected name", async () => { + const result = await runSelectionPrompt("selectForRemoval", "1\n", { applied: ["npm"] }); + expect(result.stderr).toContain("Applied presets:"); + expect(result.stderr).toContain("1) npm"); + expect(result.stderr).not.toContain("pypi"); + expect(result.selected).toBe("npm"); + }); + + it("returns null for empty input", async () => { + const result = await runSelectionPrompt("selectForRemoval", "\n", { applied: ["npm"] }); + expect(result.selected).toBeNull(); + }); + + it("rejects non-numeric input", async () => { + const result = await runSelectionPrompt("selectForRemoval", "npm\n", { + applied: ["npm"], + }); + expect(result.stderr).toContain("Invalid preset number"); + expect(result.selected).toBeNull(); + }); + + it("rejects out-of-range number", async () => { + const result = await runSelectionPrompt("selectForRemoval", "99\n", { applied: ["npm"] }); + expect(result.stderr).toContain("Invalid preset number"); + expect(result.selected).toBeNull(); + }); + + it("selects second preset when both are applied", async () => { + const result = await runSelectionPrompt("selectForRemoval", "2\n", { + applied: ["npm", "pypi"], + }); + expect(result.stderr).toContain("1) npm"); + expect(result.stderr).toContain("2) pypi"); + expect(result.selected).toBe("pypi"); + }); + + it("rejects with code EOF when stdin closes before an answer (#7418)", async () => { + const result = await runSelectionPromptAtEof("selectForRemoval", { applied: ["npm"] }); + + expect(result.stderr).toContain("Choose preset to remove:"); + expect(result.outcome).toBe("rejected"); + expect(result.code).toBe("EOF"); + }, 3_000); + + it("reports an interrupt as SIGINT rather than closed stdin (#7418)", async () => { + const result = await runSelectionPromptAtSigint("selectForRemoval", { applied: ["npm"] }); + + expect(result.code).toBe("SIGINT"); + expect(result.reraised).toBe(1); + // The signal itself, not just that kill ran: re-raising SIGTERM would + // otherwise satisfy this test. + expect(result.signal).toBe("SIGINT"); + }, 3_000); + }); + + describe("interactive prompt cleanup", () => { + it("releases and re-refs stdin around policy-add preset prompts", async () => { + const result = await runSelectionPrompt("selectFromList", "1\n"); + expect(result.selected).toBe("npm"); + expect(result.counts.ref).toBeGreaterThanOrEqual(1); + expect(result.counts.pause).toBeGreaterThanOrEqual(1); + expect(result.counts.unref).toBeGreaterThanOrEqual(1); + expect(result.close).toHaveBeenCalledOnce(); + }); + + it("releases and re-refs stdin around policy-remove preset prompts", async () => { + const result = await runSelectionPrompt("selectForRemoval", "1\n", { applied: ["npm"] }); + expect(result.selected).toBe("npm"); + expect(result.counts.ref).toBeGreaterThanOrEqual(1); + expect(result.counts.pause).toBeGreaterThanOrEqual(1); + expect(result.counts.unref).toBeGreaterThanOrEqual(1); + expect(result.close).toHaveBeenCalledOnce(); + }); + }); +});