From 4f7b5fb6023ce2ec449561782d09e3c7e5ac7c7b Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Sun, 18 Oct 2026 05:33:21 +0800 Subject: [PATCH] fix(cli): route update flag through verified updater Normalize the unambiguous root --update alias before output handling and through direct createCli().serve calls. Reuse the existing output-value scan and canonical update command, including selected environment, installation ownership, Windows-aware launch, verification, maintenance and failure facts. Retain option data and help/check precedence. Reject remaining Incur install requests: update:false does not disable its explicit installer in0.6.1. Remove the superseded output-only normalizer name and update its consumers. Validation:123focused checks/7files pass, including compiled CLI with an owned rejecting fake npm and direct serve across5formats. Original argument owner fails6new controls/7pass; isolated baseline fails8workflow checks/2controls pass. Static/docs/discovery1561 pass. Both scoped source reviews are clear. Six completed equivalent current-version workflows compare baseline canonical update with the alias; complete replies identical. Native Windows upgrade, real package-manager installation and full install costs remain unverified. The first baseline escaped a Vitest-only builtin mock and installed REA in the pinned Node global prefix. Removed the newly added package and verified original Node/npm/Corepack tools. Re-ran using the real builtin spy with synchronized ESM exports, an independently rejecting PATH shim and owned npm prefix/cache. Keep heavy tests/profiling stopped. Independent updater output bounds remain tracked in #4888 rather than changing collection policy in this routing fix. Fixes #4883 --- README.md | 3 + scripts/rea.mjs | 4 +- src/cli.ts | 13 ++ src/cliOutput.ts | 26 ++- tests/acceptance/setup/updateAliasCli.test.ts | 107 +++++++++++ tests/boundary/cli/cliOutput.test.ts | 68 ++++++- tests/boundary/cli/dispatcher.test.ts | 4 +- tests/process-global/cliUpdateAlias.test.ts | 166 ++++++++++++++++++ 8 files changed, 374 insertions(+), 17 deletions(-) create mode 100644 tests/acceptance/setup/updateAliasCli.test.ts create mode 100644 tests/process-global/cliUpdateAlias.test.ts diff --git a/README.md b/README.md index aac24a814..575cf6135 100644 --- a/README.md +++ b/README.md @@ -132,6 +132,9 @@ For a CLI installed with `npm install --global` or `bun add --global`: rea update ``` +`rea --update` is an alias for this verified workflow and accepts the same global +output flags. Use `rea update` when combining it with other command controls. + Bun global updates require Bun 1.2.17 or later. With an older Bun release, upgrade Bun manually before running `rea update`. diff --git a/scripts/rea.mjs b/scripts/rea.mjs index 540f23d00..aa0659465 100755 --- a/scripts/rea.mjs +++ b/scripts/rea.mjs @@ -74,13 +74,13 @@ if (!(await compiledRuntimeExists(runtimeFiles))) { const { createCliResultOutput } = await import("../dist/cli-runtime/cliResultOutput.js"); const { - normalizeCliOutputArguments, + normalizeCliArguments, renderCliOutputArgumentError, renderEmptyFilteredCliOutput, validateCliOutputArguments, } = await import("../dist/cli-runtime/cliOutput.js"); guardClosedStdout(); - const cliArgs = normalizeCliOutputArguments(args); + const cliArgs = normalizeCliArguments(args); const outputArguments = validateCliOutputArguments(cliArgs); const configurationError = cliConfigurationError(process.env); if (configurationError !== undefined) { diff --git a/src/cli.ts b/src/cli.ts index a12d1e327..0c4a75421 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -12,6 +12,7 @@ import { registerBinaryDiagnosticsCommands } from "./cli/binaryDiagnosticsComman import { registerFridaCommands } from "./cli/fridaCommands.js"; import type { FridaInstrumentationPort } from "./application/frida/FridaInstrumentationPort.js"; import { Cli } from "incur"; +import { normalizeCliArguments } from "./cliOutput.js"; import { createLogger, parseLogLevel } from "./logger.js"; import { projectAnalysisError } from "./domain/analysisErrorProjection.js"; @@ -94,6 +95,15 @@ export const createCli = ( "Ask what software, artifact, protocol, or behavior the user wants to understand, then choose the available investigation capabilities that can produce evidence.", }, sync: false, + update: { + // Incur's update:false only suppresses notices. Reject its explicit + // installer so ambiguous flags cannot bypass REA's verified workflow. + install: () => { + throw new Error( + "Use 'rea update' to update the identified installation. The '--update' alias accepts only global output flags.", + ); + }, + }, }); registerSetupCommands(cli, logger, environment); @@ -136,5 +146,8 @@ export const createCli = ( registerApplicationCommands(cli, logger); presentOmittableDefaults(cli); if (resultOutput !== undefined) streamCliCommandResults(cli, resultOutput); + const serve = cli.serve.bind(cli); + cli.serve = (arguments_ = process.argv.slice(2), options) => + serve(normalizeCliArguments(arguments_), options); return cli; }; diff --git a/src/cliOutput.ts b/src/cliOutput.ts index a1a6522d5..226d0be5d 100644 --- a/src/cliOutput.ts +++ b/src/cliOutput.ts @@ -11,15 +11,23 @@ const VALUED_OUTPUT_FLAGS = new Set([ "--token-offset", ]); +const ROOT_UPDATE_OUTPUT_FLAGS = new Set([ + "--json", + "--full-output", + "--token-count", +]); + /** - * Split `--flag=value` forms of the valued output flags. Incur reads these + * Route the root update alias and split `--flag=value` output forms. Incur reads these * flags only in spaced form, so `--format=json` otherwise reaches the command * parser and fails as an unknown flag. Command options accept both forms. */ -export const normalizeCliOutputArguments = ( +export const normalizeCliArguments = ( arguments_: readonly string[], ): string[] => { const normalized: string[] = []; + const updates = new Set(); + let rootUpdate = true; for (let index = 0; index < arguments_.length; index += 1) { const argument = arguments_[index]!; if (VALUED_OUTPUT_FLAGS.has(argument) && arguments_[index + 1]) { @@ -33,9 +41,17 @@ export const normalizeCliOutputArguments = ( const flag = argument.slice(0, separator); if (separator >= 0 && VALUED_OUTPUT_FLAGS.has(flag)) normalized.push(flag, argument.slice(separator + 1)); - else normalized.push(argument); + else { + if (argument === "--update") updates.add(normalized.length); + else if (!ROOT_UPDATE_OUTPUT_FLAGS.has(argument)) rootUpdate = false; + normalized.push(argument); + } } - return normalized; + // Output option values were consumed above; flag-shaped data is never an alias. + // Help/check/command/unknown tokens retain Incur's parsing and precedence. + return rootUpdate && updates.size > 0 + ? ["update", ...normalized.filter((_, index) => !updates.has(index))] + : normalized; }; export type CliOutputArgumentValidation = @@ -89,7 +105,7 @@ export const parseCliOutputArguments = ( for (let index = 0; index < arguments_.length; index += 1) { const argument = arguments_[index]; // Incur scans builtins across `--` and reads spaced value forms, which - // normalizeCliOutputArguments has produced. Mirror those effective values. + // normalizeCliArguments has produced. Mirror those effective values. if (argument === "--json") { format = "json"; continue; diff --git a/tests/acceptance/setup/updateAliasCli.test.ts b/tests/acceptance/setup/updateAliasCli.test.ts new file mode 100644 index 000000000..3dceafc90 --- /dev/null +++ b/tests/acceptance/setup/updateAliasCli.test.ts @@ -0,0 +1,107 @@ +import { mkdir, readFile, writeFile } from "node:fs/promises"; +import { delimiter, join } from "node:path"; +import { expect } from "vitest"; + +import { PRODUCT_IDENTITY } from "../../../src/identity.js"; +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; +import { cliTest } from "../../support/cli/cliFixture.js"; + +cliTest( + "root update flag uses the verified workflow and preserves executable output policies", + async ({ cli }) => { + const home = await createTestTempDirectory("rea-update-alias-"); + const bin = join(home, "bin"); + const script = join(home, "npm-fixture.mjs"); + const log = join(home, "commands.jsonl"); + await mkdir(bin); + await writeFile(log, ""); + await writeFile( + script, + ` + import {appendFileSync} from 'node:fs'; + import {join} from 'node:path'; + const args=process.argv.slice(2); + appendFileSync(process.env.REA_TEST_NPM_LOG, JSON.stringify(args)+'\\n'); + if(args[0]==='root') console.log(join(process.env.USERPROFILE,'unowned','node_modules')); + else if(args[0]==='prefix') console.log(join(process.env.USERPROFILE,'unowned')); + else if(args[0]==='view') { + if(process.env.REA_TEST_NPM_FAILURE==='1') {process.stderr.write('owned fake registry denied');process.exitCode=2;} + else console.log(JSON.stringify(${JSON.stringify(PRODUCT_IDENTITY.packageVersion)})); + } else {process.stderr.write('No installation or unexpected package-manager commands are allowed');process.exitCode=97;} + `, + ); + await writeFile( + join(bin, process.platform === "win32" ? "npm.cmd" : "npm"), + process.platform === "win32" + ? '@echo off\r\n"%REA_TEST_NODE%" "%REA_TEST_NPM_SCRIPT%" %*\r\n' + : '#!/bin/sh\nexec "$REA_TEST_NODE" "$REA_TEST_NPM_SCRIPT" "$@"\n', + { mode: 0o700 }, + ); + const run = (arguments_: readonly string[], failure = false) => + cli.run({ + arguments: arguments_, + cwd: home, + timeoutMs: 20_000, + environment: { + USERPROFILE: home, + PATH: bin + delimiter + (process.env.PATH ?? ""), + REA_TEST_NODE: process.execPath, + REA_TEST_NPM_SCRIPT: script, + REA_TEST_NPM_LOG: log, + REA_TEST_NPM_FAILURE: failure ? "1" : "0", + CI: "1", + NO_UPDATE_NOTIFIER: "1", + }, + }); + const canonical = await run(["update", "--json"]); + expect(canonical.exitCode, canonical.stderr).toBe(0); + expect(canonical.json).toMatchObject({ + status: "current", + currentVersion: PRODUCT_IDENTITY.packageVersion, + latestVersion: PRODUCT_IDENTITY.packageVersion, + }); + const alias = await run(["--json", "--update"]); + expect(alias.exitCode, alias.stderr).toBe(0); + expect(alias.json).toEqual(canonical.json); + const filtered = await run([ + "--update", + "--format=json", + "--full-output", + "--filter-output=status", + ]); + expect(filtered.exitCode, filtered.stderr).toBe(0); + expect(filtered.json).toMatchObject({ + ok: true, + data: "current", + meta: { command: expect.stringContaining("update") }, + }); + const failed = await run(["--update", "--json"], true); + expect(failed.exitCode).toBe(1); + expect(failed.json).toMatchObject({ + status: "failed", + reason: "version-check", + remediation: expect.stringContaining("owned fake registry denied"), + }); + const before = await readFile(log, "utf8"); + const ambiguous = await run(["providers", "--update", "--json"]); + expect(ambiguous.exitCode).toBe(1); + expect(ambiguous.json).toMatchObject({ + code: "UPDATE_FAILED", + message: expect.stringContaining("rea update"), + }); + expect(await readFile(log, "utf8")).toBe(before); + const help = await run(["--update", "--help"]); + expect(help.exitCode, help.stderr).toBe(0); + expect(help.stdout).toContain("update"); + expect(await readFile(log, "utf8")).toBe(before); + const commands: unknown[] = before + .trim() + .split("\n") + .map((line) => JSON.parse(line)); + expect(commands.length).toBeGreaterThan(0); + expect(commands).toEqual( + expect.arrayContaining([expect.arrayContaining(["view"])]), + ); + for (const command of commands) expect(command).not.toContain("install"); + }, +); diff --git a/tests/boundary/cli/cliOutput.test.ts b/tests/boundary/cli/cliOutput.test.ts index e26b6d84e..49ccb9137 100644 --- a/tests/boundary/cli/cliOutput.test.ts +++ b/tests/boundary/cli/cliOutput.test.ts @@ -7,7 +7,7 @@ import { describe, expect, it } from "vitest"; import { workspaceCliTest } from "../../support/cli/workspaceCliFixture.js"; import { - normalizeCliOutputArguments, + normalizeCliArguments, renderCliOutputArgumentError, renderEmptyFilteredCliOutput, validateCliOutputArguments, @@ -64,12 +64,7 @@ describe("CLI output flag value forms", () => { "preserves a flag-shaped spaced value owned by %s", (flag) => { expect( - normalizeCliOutputArguments([ - flag, - "--format=json", - "providers", - "--json", - ]), + normalizeCliArguments([flag, "--format=json", "providers", "--json"]), ).toEqual([flag, "--format=json", "providers", "--json"]); }, ); @@ -95,7 +90,7 @@ describe("CLI output flag value forms", () => { it("splits only the equals forms of valued output flags", () => { expect( - normalizeCliOutputArguments([ + normalizeCliArguments([ "inspect-sqlite-database", "--table=a=b", "--format=json", @@ -541,3 +536,60 @@ describe("compiled CLI artifact diagnostics", () => { CLI_INTEGRATION_TIMEOUT_MS, ); }); + +describe("CLI update root alias", () => { + it.each([ + [["--update"], ["update"]], + [ + ["--json", "--update", "--filter-output=status"], + ["update", "--json", "--filter-output", "status"], + ], + [ + ["--format=yaml", "--update", "--full-output"], + ["update", "--format", "yaml", "--full-output"], + ], + [ + ["--update", "--token-limit=100", "--token-count"], + ["update", "--token-limit", "100", "--token-count"], + ], + [["--update", "--update"], ["update"]], + [ + ["--filter-output", "--update"], + ["--filter-output", "--update"], + ], + [ + ["--filter-output=--update", "--update"], + ["update", "--filter-output", "--update"], + ], + [ + ["--update", "--help"], + ["--update", "--help"], + ], + [ + ["--incur-update-check", "--update"], + ["--incur-update-check", "--update"], + ], + [ + ["providers", "--update"], + ["providers", "--update"], + ], + [ + ["--", "--update"], + ["--", "--update"], + ], + [ + ["--config", "--update"], + ["--config", "--update"], + ], + [ + ["update", "--json"], + ["update", "--json"], + ], + ])( + "routes only an unambiguous root update while retaining option data: %j", + (input, expected) => { + expect(normalizeCliArguments(input)).toEqual(expected); + expect(normalizeCliArguments(expected)).toEqual(expected); + }, + ); +}); diff --git a/tests/boundary/cli/dispatcher.test.ts b/tests/boundary/cli/dispatcher.test.ts index 35c9114c5..516d33b68 100644 --- a/tests/boundary/cli/dispatcher.test.ts +++ b/tests/boundary/cli/dispatcher.test.ts @@ -23,7 +23,7 @@ describe("executable dispatcher", () => { ), workspace.write( "dist/cli-runtime/cliOutput.js", - "export const normalizeCliOutputArguments = (args) => args; export const validateCliOutputArguments = () => ({ ok: true }); export const renderCliOutputArgumentError = () => ''; export const renderEmptyFilteredCliOutput = () => undefined;\n", + "export const normalizeCliArguments = (args) => args; export const validateCliOutputArguments = () => ({ ok: true }); export const renderCliOutputArgumentError = () => ''; export const renderEmptyFilteredCliOutput = () => undefined;\n", ), workspace.write( "dist/cli-runtime/cliResultOutput.js", @@ -64,7 +64,7 @@ describe("executable dispatcher", () => { ), workspace.write( "dist/cli-runtime/cliOutput.js", - "export const normalizeCliOutputArguments = (args) => args; export const validateCliOutputArguments = () => ({ ok: true }); export const renderCliOutputArgumentError = () => ''; export const renderEmptyFilteredCliOutput = () => undefined;\n", + "export const normalizeCliArguments = (args) => args; export const validateCliOutputArguments = () => ({ ok: true }); export const renderCliOutputArgumentError = () => ''; export const renderEmptyFilteredCliOutput = () => undefined;\n", ), workspace.write( "dist/cli-runtime/cliResultOutput.js", diff --git a/tests/process-global/cliUpdateAlias.test.ts b/tests/process-global/cliUpdateAlias.test.ts new file mode 100644 index 000000000..92b750df1 --- /dev/null +++ b/tests/process-global/cliUpdateAlias.test.ts @@ -0,0 +1,166 @@ +import childProcess from "node:child_process"; +import { syncBuiltinESMExports } from "node:module"; +import { writeFile } from "node:fs/promises"; +import { join } from "node:path"; +import { afterEach, beforeEach, expect, it, vi } from "vitest"; +import { createTestTempDirectory } from "../fixtures/temporaryDirectory.js"; + +import { createCli } from "../../src/cli.js"; +import { PRODUCT_IDENTITY } from "../../src/identity.js"; + +const mocks = vi.hoisted(() => ({ + run: vi.fn(), + host: vi.fn(() => ({ owner: "selected-update-host" })), + dependencyInstall: vi.fn(() => { + throw new Error("Dependency installer must not run in this test."); + }), +})); + +vi.mock("../../src/application/Update.js", async (original) => ({ + ...(await original()), + runUpdate: mocks.run, +})); +vi.mock("../../src/application/UpdateRuntime.js", () => ({ + systemUpdateHost: mocks.host, +})); + +const initialExitCode = process.exitCode; +const current = { + status: "current", + currentVersion: PRODUCT_IDENTITY.packageVersion, + latestVersion: PRODUCT_IDENTITY.packageVersion, + installMethod: "unknown", +}; + +beforeEach(async () => { + vi.clearAllMocks(); + const root = await createTestTempDirectory("rea-update-api-sandbox-"); + await writeFile( + join(root, process.platform === "win32" ? "npm.cmd" : "npm"), + process.platform === "win32" + ? "@echo off\r\necho Test package manager refuses installation 1>&2\r\nexit /b 97\r\n" + : "#!/bin/sh\nprintf '%s\\n' 'Test package manager refuses installation' >&2\nexit 97\n", + { mode: 0o700 }, + ); + vi.stubEnv("PATH", root); + vi.stubEnv("npm_config_user_agent", "npm/test"); + vi.stubEnv("npm_config_prefix", join(root, "global")); + vi.stubEnv("npm_config_cache", join(root, "cache")); + // Externalized dependencies bypass Vitest module factories for Node builtins. + // Patch the real builtin and synchronize named ESM exports; PATH/prefix above + // independently prevent any real installation if this guard stops working. + vi.spyOn(childProcess, "execFile").mockImplementation( + mocks.dependencyInstall, + ); + syncBuiltinESMExports(); + vi.stubEnv("CI", "1"); + vi.stubEnv("NO_UPDATE_NOTIFIER", "1"); + vi.stubGlobal( + "fetch", + vi.fn(() => Promise.reject(new Error("No network in this test."))), + ); + mocks.run.mockResolvedValue(current); +}); +afterEach(() => { + vi.restoreAllMocks(); + syncBuiltinESMExports(); + vi.unstubAllEnvs(); + vi.unstubAllGlobals(); + process.exitCode = initialExitCode; +}); + +const serve = async ( + cli: ReturnType, + arguments_: string[], +) => { + process.exitCode = 0; + let stdout = ""; + let exitCode = 0; + await cli.serve(arguments_, { + env: {}, + stdout: (text) => { + stdout += text; + }, + exit: (code) => { + exitCode = code; + }, + }); + return { stdout, exitCode: exitCode || Number(process.exitCode ?? 0) }; +}; + +it.each(["json", "jsonl", "yaml", "toon", "md"])( + "uses the verified update command and selected environment through direct serve with %s output", + async (format) => { + const selected = { REA_SELECTED_UPDATE: "original" }; + const cli = createCli(selected); + selected.REA_SELECTED_UPDATE = "changed"; + const expected = await serve(cli, ["update", "--format", format]); + const actual = await serve(cli, ["--format", format, "--update"]); + expect(actual).toEqual(expected); + expect(actual.exitCode).toBe(0); + expect(actual.stdout).toContain(PRODUCT_IDENTITY.packageVersion); + expect(mocks.run).toHaveBeenCalledTimes(2); + expect(mocks.run).toHaveBeenLastCalledWith( + PRODUCT_IDENTITY.packageVersion, + { owner: "selected-update-host" }, + "structured", + ); + expect(mocks.host).toHaveBeenLastCalledWith(undefined, undefined, { + REA_SELECTED_UPDATE: "original", + }); + expect(mocks.dependencyInstall).not.toHaveBeenCalled(); + }, +); + +it("preserves filtering, full output and unsuccessful update exit status", async () => { + const cli = createCli({}); + const flags = ["--json", "--full-output", "--filter-output", "status"]; + const filtered = await serve(cli, ["--update", ...flags]); + const canonical = await serve(cli, ["update", ...flags]); + expect(filtered.exitCode).toBe(canonical.exitCode); + for (const output of [filtered, canonical]) + expect(JSON.parse(output.stdout)).toMatchObject({ + ok: true, + data: "current", + meta: { command: expect.stringContaining("update") }, + }); + mocks.run.mockResolvedValue({ + status: "failed", + currentVersion: PRODUCT_IDENTITY.packageVersion, + latestVersion: "99.0.0", + reason: "install", + remediation: "EACCES owned installation", + }); + const failed = await serve(cli, ["--update", "--json"]); + expect(failed.exitCode).toBe(1); + expect(JSON.parse(failed.stdout)).toMatchObject({ + status: "failed", + reason: "install", + remediation: "EACCES owned installation", + }); + expect(mocks.dependencyInstall).not.toHaveBeenCalled(); +}); + +it.each([ + ["--update", "--help"], + ["--incur-update-check", "--update"], +])("preserves non-installing precedence for %j", async (...arguments_) => { + await serve(createCli({}), arguments_); + expect(mocks.run).not.toHaveBeenCalled(); + expect(mocks.dependencyInstall).not.toHaveBeenCalled(); +}); + +it("refuses ambiguous built-in installation rather than selecting another updater", async () => { + const result = await serve(createCli({}), [ + "providers", + "--update", + "--json", + ]); + expect(result.exitCode).toBe(1); + expect(JSON.parse(result.stdout)).toMatchObject({ + code: "UPDATE_FAILED", + message: expect.stringContaining("rea update"), + }); + expect(mocks.run).not.toHaveBeenCalled(); + expect(mocks.dependencyInstall).not.toHaveBeenCalled(); +});