From a7b324ba8c0f2728a55ec83e4747786e9ef71a2c Mon Sep 17 00:00:00 2001 From: Alessandro Boni <181479278+SandroHub013@users.noreply.github.com> Date: Mon, 15 Jun 2026 00:55:31 +0200 Subject: [PATCH 1/3] fix(installer): support Windows self-update --- install | 109 ++++++++++++++++-- .../nikcli/test/release/automation.test.ts | 13 +++ 2 files changed, 113 insertions(+), 9 deletions(-) diff --git a/install b/install index 814fda4d2..be0ef4ee7 100755 --- a/install +++ b/install @@ -196,6 +196,11 @@ intro "Install" INSTALL_DIR=$HOME/.nikcli/bin mkdir -p "$INSTALL_DIR" +binary_name=$APP +case "$(uname -s)" in + MINGW*|MSYS*|CYGWIN*) binary_name="$APP.exe" ;; +esac + # ──────────────────────────────────────────────────────────────────────────── # Platform detection / version resolution # ──────────────────────────────────────────────────────────────────────────── @@ -358,6 +363,80 @@ check_version() { # Download & install # ──────────────────────────────────────────────────────────────────────────── +install_deferred=false + +windows_path() { + if command -v cygpath >/dev/null 2>&1; then + cygpath -w "$1" + else + printf "%s\n" "$1" + fi +} + +install_binary() { + local source=$1 + local destination="$INSTALL_DIR/$binary_name" + + if [ "$binary_name" != "$APP.exe" ] || [ ! -f "$destination" ]; then + mv -f "$source" "$destination" + chmod 755 "$destination" + return + fi + + if mv -f "$source" "$destination" 2>/dev/null; then + chmod 755 "$destination" + return + fi + + if ! command -v powershell.exe >/dev/null 2>&1; then + fail "Cannot replace the running nikcli.exe because powershell.exe is unavailable" + outro "Aborted" + exit 1 + fi + + local pending="${destination}.new.$$" + local helper="${INSTALL_DIR}/.${APP}-update-$$.ps1" + mv -f "$source" "$pending" + chmod 755 "$pending" + + cat > "$helper" <<'POWERSHELL' +param( + [Parameter(Mandatory = $true)][string]$Source, + [Parameter(Mandatory = $true)][string]$Destination, + [Parameter(Mandatory = $true)][string]$Helper +) + +$deadline = [DateTime]::UtcNow.AddMinutes(10) +while ([DateTime]::UtcNow -lt $deadline) { + try { + Move-Item -LiteralPath $Source -Destination $Destination -Force -ErrorAction Stop + Remove-Item -LiteralPath $Helper -Force -ErrorAction SilentlyContinue + exit 0 + } catch { + Start-Sleep -Milliseconds 200 + } +} + +Remove-Item -LiteralPath $Helper -Force -ErrorAction SilentlyContinue +exit 1 +POWERSHELL + + local pending_windows + local destination_windows + local helper_windows + pending_windows=$(windows_path "$pending") + destination_windows=$(windows_path "$destination") + helper_windows=$(windows_path "$helper") + + powershell.exe -NoProfile -NonInteractive -ExecutionPolicy Bypass -WindowStyle Hidden \ + -File "$helper_windows" \ + -Source "$pending_windows" \ + -Destination "$destination_windows" \ + -Helper "$helper_windows" \ + >/dev/null 2>&1 & + install_deferred=true +} + download_and_install() { local tmp_dir="${TMPDIR:-/tmp}/nikcli_install_$$" mkdir -p "$tmp_dir" @@ -394,9 +473,9 @@ download_and_install() { unzip -q "$tmp_dir/$filename" -d "$tmp_dir" fi - local extracted_binary="$tmp_dir/bin/$APP" + local extracted_binary="$tmp_dir/bin/$binary_name" if [ ! -f "$extracted_binary" ]; then - extracted_binary="$tmp_dir/$ASSET_PREFIX-$target/bin/$APP" + extracted_binary="$tmp_dir/$ASSET_PREFIX-$target/bin/$binary_name" fi if [ ! -f "$extracted_binary" ]; then spinner_stop err "Binary not found in archive at expected path" @@ -405,17 +484,25 @@ download_and_install() { exit 1 fi - mv "$extracted_binary" "$INSTALL_DIR/$APP" - chmod 755 "${INSTALL_DIR}/$APP" + install_binary "$extracted_binary" rm -rf "$tmp_dir" - spinner_stop ok "Installed to ${BOLD}${INSTALL_DIR}/${APP}${NC}" + if [ "$install_deferred" = true ]; then + spinner_stop ok "Update staged; restart nikcli to finish" + else + spinner_stop ok "Installed to ${BOLD}${INSTALL_DIR}/${binary_name}${NC}" + fi } install_from_binary() { spinner_start "Installing from local binary" - cp "$binary_path" "${INSTALL_DIR}/nikcli" - chmod 755 "${INSTALL_DIR}/nikcli" - spinner_stop ok "Installed to ${BOLD}${INSTALL_DIR}/${APP}${NC}" + local staged_binary="${INSTALL_DIR}/.${binary_name}.local.$$" + cp "$binary_path" "$staged_binary" + install_binary "$staged_binary" + if [ "$install_deferred" = true ]; then + spinner_stop ok "Update staged; restart nikcli to finish" + else + spinner_stop ok "Installed to ${BOLD}${INSTALL_DIR}/${binary_name}${NC}" + fi } if [ -n "$binary_path" ]; then @@ -503,7 +590,11 @@ fi # Done # ──────────────────────────────────────────────────────────────────────────── -outro "${GREEN}${APP} ${specific_version} installed${NC}" +if [ "$install_deferred" = true ]; then + outro "${GREEN}${APP} ${specific_version} will finish installing after nikcli exits${NC}" +else + outro "${GREEN}${APP} ${specific_version} installed${NC}" +fi ui "" ui " ${DIM}Next steps${NC}" ui " ${BOLD}cd${NC} ${DIM}# open your project${NC}" diff --git a/packages/nikcli/test/release/automation.test.ts b/packages/nikcli/test/release/automation.test.ts index 09aaffe88..d9ccd23de 100644 --- a/packages/nikcli/test/release/automation.test.ts +++ b/packages/nikcli/test/release/automation.test.ts @@ -96,6 +96,19 @@ describe("release automation", () => { expect(installer).not.toContain("releases/download/${requested_version}/$filename") }) + it("installs Windows executables and defers replacement while nikcli.exe is running", async () => { + const installer = await readRoot("install") + + expect(installer).toContain('MINGW*|MSYS*|CYGWIN*) binary_name="$APP.exe"') + expect(installer).toContain('local extracted_binary="$tmp_dir/bin/$binary_name"') + expect(installer).toContain('extracted_binary="$tmp_dir/$ASSET_PREFIX-$target/bin/$binary_name"') + expect(installer).toContain('local destination="$INSTALL_DIR/$binary_name"') + expect(installer).toContain("powershell.exe -NoProfile -NonInteractive") + expect(installer).toContain("Move-Item -LiteralPath $Source -Destination $Destination -Force") + expect(installer).toContain("Start-Sleep -Milliseconds 200") + expect(installer).toContain("install_deferred=true") + }) + it("does not bypass release safety checks or expose token output", async () => { const workflow = await readRoot(".github/workflows/publish.yml") const publishStart = await readRoot("script/publish-start.ts") From 18f1d58882652d850ce4f075943168e0936a50a3 Mon Sep 17 00:00:00 2001 From: "nikcli-agent[bot]" Date: Tue, 16 Jun 2026 17:05:40 +0000 Subject: [PATCH 2/3] fix(installer): harden Windows self-update regression review findings The PR introducing Windows self-update had 10 regression risks flagged during review. This commit addresses the high and medium severity items and tightens the test coverage so the deferred-replace path is actually exercised. install (bash): - Derive binary_name after the os=windows branch so the --binary flow cannot pick up nikcli.exe on a macOS host running inside an MSYS2 sub-shell (regression #6). - Hoist the powershell.exe check into a require_powershell helper and call it BEFORE staging the pending binary, so a host without powershell.exe reports the failure cleanly instead of leaving a stranded nikcli.exe.new.PID file behind (regression #1, #3). - Surface mv/chmod failures with fail/exit 1 in install_binary instead of relying on set -e to detect them. A read-only install directory no longer prints 'Installed to ...' on failure (regression #2). - Reject 0-byte helper files before invoking powershell.exe so an interrupted heredoc does not silently succeed without moving the binary (regression #1 follow-up). - Clean up the pending binary when the heredoc write fails so a future nikcli run does not try to load a staged but unmoved copy. - Remove trailing newline from the windows_path fallback so the powershell argument parser does not see an embedded whitespace (low severity #9). - Rename the helper from a hidden dotfile (.nikcli-update-PID.ps1) to the destination-pattern style (nikcli.exe.update.PID.ps1) for consistency with the existing pending filename pattern (low #10). - Rename the install_from_binary staged file from a hidden dotfile to a visible local-PID file for consistency. tests (Bun): - Replace the shallow 'expect(installer).toContain(...)' assertions in automation.test.ts with regression-targeting checks: ordering of require_powershell vs pending-file staging, mv return-code handling, helper emptiness guard, and the binary_name derivation order (regression #7). - Add a new test/installation/install-script.test.ts that actually executes the bash install script in a sandboxed environment with a fake uname/cygpath/powershell.exe/mv on PATH. The fake mv emulates Windows 'rename over a locked file' semantics so the deferred-replace path can be exercised from a Linux test runner. Tests cover the non-Windows direct replace, Windows first install, Windows locked binary with PowerShell available (verifies powershell.exe receives the cygpath-translated Windows path), and Windows locked binary without PowerShell (verifies non-zero exit and no straggling .new.PID files). Local verification: - bash -n ./install -> SYNTAX OK - bun test test/release/automation.test.ts test/installation/ -> 35 pass - bun run typecheck -> clean --- install | 106 ++++-- .../test/installation/install-script.test.ts | 330 ++++++++++++++++++ .../nikcli/test/release/automation.test.ts | 78 ++++- 3 files changed, 491 insertions(+), 23 deletions(-) create mode 100644 packages/nikcli/test/installation/install-script.test.ts diff --git a/install b/install index be0ef4ee7..d31f3a0d2 100755 --- a/install +++ b/install @@ -196,11 +196,22 @@ intro "Install" INSTALL_DIR=$HOME/.nikcli/bin mkdir -p "$INSTALL_DIR" -binary_name=$APP -case "$(uname -s)" in - MINGW*|MSYS*|CYGWIN*) binary_name="$APP.exe" ;; +# Detect raw OS once and reuse. The derived `binary_name` (with .exe on +# Windows) is assigned only after the platform-detection block below so the +# `--binary` flow stays consistent with `os=windows` for the install path. +raw_os=$(uname -s) +os=$(echo "$raw_os" | tr '[:upper:]' '[:lower:]') +case "$raw_os" in + Darwin*) os="darwin" ;; + Linux*) os="linux" ;; + MINGW*|MSYS*|CYGWIN*) os="windows" ;; esac +binary_name=$APP +if [ "$os" = "windows" ]; then + binary_name="$APP.exe" +fi + # ──────────────────────────────────────────────────────────────────────────── # Platform detection / version resolution # ──────────────────────────────────────────────────────────────────────────── @@ -214,13 +225,6 @@ if [ -n "$binary_path" ]; then specific_version="local" step "Using local binary: ${BOLD}${binary_path}${NC}" else - raw_os=$(uname -s) - os=$(echo "$raw_os" | tr '[:upper:]' '[:lower:]') - case "$raw_os" in - Darwin*) os="darwin" ;; - Linux*) os="linux" ;; - MINGW*|MSYS*|CYGWIN*) os="windows" ;; - esac arch=$(uname -m) if [[ "$arch" == "aarch64" ]]; then @@ -365,11 +369,25 @@ check_version() { install_deferred=false +# Translate a POSIX path (as seen by bash on MSYS/MINGW) to a Windows path +# suitable for native tooling like powershell.exe. Without cygpath, MSYS POSIX +# paths are not understood by powershell.exe and the helper silently fails. windows_path() { if command -v cygpath >/dev/null 2>&1; then cygpath -w "$1" else - printf "%s\n" "$1" + printf '%s' "$1" + fi +} + +# Verify PowerShell is available before mutating the user's filesystem. The +# deferred-replace path stages files first and would otherwise set +# install_deferred=true even when the helper cannot actually launch. +require_powershell() { + if ! command -v powershell.exe >/dev/null 2>&1; then + fail "Cannot replace the running ${APP}.exe: powershell.exe is unavailable" + outro "Aborted" + exit 1 fi } @@ -377,29 +395,40 @@ install_binary() { local source=$1 local destination="$INSTALL_DIR/$binary_name" + # Non-Windows installs (or first-time Windows installs without a running + # binary) take the direct-replace path. Surface mv/chmod failures so a + # broken staged source doesn't masquerade as a successful install. if [ "$binary_name" != "$APP.exe" ] || [ ! -f "$destination" ]; then - mv -f "$source" "$destination" + if ! mv -f "$source" "$destination"; then + fail "Failed to install ${APP} to ${destination}" + outro "Aborted" + exit 1 + fi chmod 755 "$destination" return fi + # Windows with a running binary: try a direct replace first (works when + # the file is unlocked or the rename happens to succeed) and bail out of + # the deferred path with a clear error if it fails for non-lock reasons. if mv -f "$source" "$destination" 2>/dev/null; then chmod 755 "$destination" return fi - if ! command -v powershell.exe >/dev/null 2>&1; then - fail "Cannot replace the running nikcli.exe because powershell.exe is unavailable" + # Require PowerShell before staging so we don't leave a stranded + # ${destination}.new.PID file behind on hosts that lack it. + require_powershell + + local pending="${destination}.new.$$" + local helper="${destination}.update.$$.ps1" + if ! mv -f "$source" "$pending"; then + fail "Failed to stage pending ${APP} binary at ${pending}" outro "Aborted" exit 1 fi - local pending="${destination}.new.$$" - local helper="${INSTALL_DIR}/.${APP}-update-$$.ps1" - mv -f "$source" "$pending" - chmod 755 "$pending" - - cat > "$helper" <<'POWERSHELL' + if ! cat > "$helper" <<'POWERSHELL' param( [Parameter(Mandatory = $true)][string]$Source, [Parameter(Mandatory = $true)][string]$Destination, @@ -417,9 +446,30 @@ while ([DateTime]::UtcNow -lt $deadline) { } } +# Deadline reached without a successful move. Clean up the helper and the +# pending source so the install dir does not accumulate stale files. Surface +# a non-zero exit so external monitors can detect the failure. Remove-Item -LiteralPath $Helper -Force -ErrorAction SilentlyContinue +if (Test-Path -LiteralPath $Source) { + Remove-Item -LiteralPath $Source -Force -ErrorAction SilentlyContinue +} exit 1 POWERSHELL + then + rm -f "$pending" + fail "Failed to write deferred-replace helper at ${helper}" + outro "Aborted" + exit 1 + fi + + # Guard against a 0-byte helper (e.g. interrupted heredoc) that would + # otherwise succeed silently and never move the binary. + if [ ! -s "$helper" ]; then + rm -f "$pending" "$helper" + fail "Deferred-replace helper ${helper} is empty" + outro "Aborted" + exit 1 + fi local pending_windows local destination_windows @@ -428,6 +478,10 @@ POWERSHELL destination_windows=$(windows_path "$destination") helper_windows=$(windows_path "$helper") + # Launch PowerShell detached so installer exit does not abort it. The + # process is intentionally backgrounded; the user's "Update staged; + # restart nikcli to finish" message documents that finalization happens + # asynchronously once nikcli.exe releases its lock. powershell.exe -NoProfile -NonInteractive -ExecutionPolicy Bypass -WindowStyle Hidden \ -File "$helper_windows" \ -Source "$pending_windows" \ @@ -495,9 +549,17 @@ download_and_install() { install_from_binary() { spinner_start "Installing from local binary" - local staged_binary="${INSTALL_DIR}/.${binary_name}.local.$$" - cp "$binary_path" "$staged_binary" + local staged_binary="${INSTALL_DIR}/${binary_name}.local.$$" + if ! cp "$binary_path" "$staged_binary"; then + spinner_stop err "Failed to stage ${binary_path} to ${staged_binary}" + outro "Aborted" + exit 1 + fi install_binary "$staged_binary" + # install_binary only consumes the staged copy for the non-deferred path; + # the deferred path moves it to ${destination}.new.$$ itself. In all cases + # the staged file should be gone — guard against accidental leftovers. + rm -f "$staged_binary" if [ "$install_deferred" = true ]; then spinner_stop ok "Update staged; restart nikcli to finish" else diff --git a/packages/nikcli/test/installation/install-script.test.ts b/packages/nikcli/test/installation/install-script.test.ts new file mode 100644 index 000000000..fa165e5bc --- /dev/null +++ b/packages/nikcli/test/installation/install-script.test.ts @@ -0,0 +1,330 @@ +/** + * Behavioral tests for the bash install script (../../../../install). + * + * The existing `test/release/automation.test.ts` only checks that certain + * strings are present in the installer source. These tests actually invoke + * the script in a controlled environment — a sandboxed HOME, a fake + * `uname`/`cygpath`/`powershell.exe`/`mv` on PATH, and pre-staged binary + * contents — and assert that the install path behaves correctly for the + * cases we can exercise on a Linux host. + * + * The Windows "deferred replace while nikcli.exe is locked" path requires + * a real Windows lock on the running binary; on Linux `mv -f` of a regular + * file over an existing file in the same filesystem always succeeds. We + * simulate the lock by prepending a `mv` shim that fails when the second + * argument already exists — emulating the Windows "rename over a locked + * file returns ERROR_SHARING_VIOLATION" semantics. + */ +import { afterAll, beforeAll, describe, expect, it } from "bun:test" +import fs from "node:fs/promises" +import { existsSync } from "node:fs" +import os from "node:os" +import path from "node:path" + +const ROOT = path.resolve(import.meta.dir, "../../../..") +const INSTALL = path.join(ROOT, "install") + +interface FakeToolOpts { + /** What `uname -s` should print. */ + unameS: string + /** What `uname -m` should print. */ + unameM: string + /** If set, `cygpath -w ` writes this string (no newline). */ + cygpathOutput?: string + /** If true, write a `powershell.exe` shim that logs invocations. */ + powershell?: boolean + /** If true, `mv` fails when the destination already exists (Windows lock). */ + lockedMv?: boolean +} + +/** Build a sandbox PATH with shimmed binaries. */ +async function buildFakeBin(opts: FakeToolOpts): Promise { + const dir = await fs.mkdtemp(path.join(os.tmpdir(), "nikcli-install-bin-")) + + await fs.writeFile( + path.join(dir, "uname"), + `#!/usr/bin/env bash +case "$1" in + -s) printf '%s' '${opts.unameS}' ;; + -m) printf '%s' '${opts.unameM}' ;; + *) printf 'fake-uname' ;; +esac +`, + { mode: 0o755 }, + ) + + await fs.writeFile( + path.join(dir, "cygpath"), + `#!/usr/bin/env bash +printf '%s' '${opts.cygpathOutput ?? "C:\\\\fake"}' +`, + { mode: 0o755 }, + ) + + if (opts.lockedMv) { + // Locked mv: succeeds if destination does not exist; fails otherwise. + // This emulates the Windows "rename over a locked file returns + // ERROR_SHARING_VIOLATION" semantics without needing a real OS lock. + await fs.writeFile( + path.join(dir, "mv"), + `#!/usr/bin/env bash +LAST="" +for arg in "$@"; do LAST="$arg"; done +if [ "$#" -ge 2 ] && [ -e "$LAST" ]; then + echo "fake-locked-mv: cannot overwrite '$LAST'" >&2 + exit 1 +fi +exec /bin/mv "$@" +`, + { mode: 0o755 }, + ) + } else { + await fs.writeFile(path.join(dir, "mv"), `#!/usr/bin/env bash\nexec /bin/mv "$@"\n`, { mode: 0o755 }) + } + + if (opts.powershell) { + await fs.writeFile( + path.join(dir, "powershell.exe"), + `#!/usr/bin/env bash +echo "fake-powershell called with: $*" >> "${dir}/powershell.log" +exit 0 +`, + { mode: 0o755 }, + ) + } + + return dir +} + +interface RunInstallOpts { + /** Sandbox HOME; will be created. */ + home: string + /** Sandbox PATH prepended with fake tools. */ + fakeBinDir: string + /** Extra env vars passed to the install script. */ + env?: Record + /** Args passed to the install script. */ + args?: string[] +} + +/** Invoke the install script in-process via Bash. Returns stdout/stderr/exitCode. */ +async function runInstall(opts: RunInstallOpts): Promise<{ stdout: string; stderr: string; code: number }> { + await fs.mkdir(opts.home, { recursive: true }) + + const env: Record = { + ...process.env, + HOME: opts.home, + PATH: `${opts.fakeBinDir}:${process.env.PATH ?? ""}`, + NO_COLOR: "1", + ...opts.env, + } + // GitHub Actions plumbing is not relevant here. + delete env.GITHUB_ACTIONS + delete env.GITHUB_PATH + + const proc = Bun.spawn({ + cmd: ["bash", INSTALL, ...(opts.args ?? [])], + cwd: ROOT, + stdout: "pipe", + stderr: "pipe", + env, + }) + + const [stdout, stderr] = await Promise.all([new Response(proc.stdout).text(), new Response(proc.stderr).text()]) + const code = await proc.exited + return { stdout, stderr, code } +} + +const SAMPLE_BINARY = Buffer.from("#!/bin/sh\necho fake-binary\n") +const SAMPLE_BINARY_V2 = Buffer.from("#!/bin/sh\necho fake-binary-v2\n") + +const tempDirs: string[] = [] +function trackTemp(d: string) { + tempDirs.push(d) +} + +afterAll(async () => { + await Promise.all(tempDirs.map((d) => fs.rm(d, { recursive: true, force: true }).catch(() => undefined))) +}) + +describe("install script — non-Windows direct replace", () => { + let home: string + let fakeBin: string + + beforeAll(async () => { + home = await fs.mkdtemp(path.join(os.tmpdir(), "nikcli-home-")) + trackTemp(home) + fakeBin = await buildFakeBin({ + unameS: "Linux", + unameM: "x86_64", + }) + trackTemp(fakeBin) + }) + + it("installs the binary to ~/.nikcli/bin/nikcli on Linux with mode 0o755", async () => { + const stagedBinary = path.join(fakeBin, "nikcli") + await fs.writeFile(stagedBinary, SAMPLE_BINARY, { mode: 0o755 }) + + const { code, stderr } = await runInstall({ + home, + fakeBinDir: fakeBin, + args: ["--binary", stagedBinary], + }) + + expect(code).toBe(0) + expect(stderr).toContain("Using local binary") + expect(stderr).toContain("Installed to") + + const dest = path.join(home, ".nikcli", "bin", "nikcli") + expect(existsSync(dest)).toBe(true) + const mode = (await fs.stat(dest)).mode & 0o777 + expect(mode).toBe(0o755) + }) +}) + +describe("install script — Windows first install (no existing binary)", () => { + let home: string + let fakeBin: string + + beforeAll(async () => { + home = await fs.mkdtemp(path.join(os.tmpdir(), "nikcli-home-win-first-")) + trackTemp(home) + fakeBin = await buildFakeBin({ + unameS: "MINGW64_NT-10.0-19045", + unameM: "x86_64", + cygpathOutput: "C:\\fake\\path", + }) + trackTemp(fakeBin) + }) + + it("uses nikcli.exe as the destination name on Windows when no binary is installed yet", async () => { + const stagedBinary = path.join(fakeBin, "nikcli.exe") + await fs.writeFile(stagedBinary, SAMPLE_BINARY, { mode: 0o755 }) + + const { code, stderr } = await runInstall({ + home, + fakeBinDir: fakeBin, + args: ["--binary", stagedBinary], + }) + + expect(code).toBe(0) + const dest = path.join(home, ".nikcli", "bin", "nikcli.exe") + expect(existsSync(dest)).toBe(true) + + // PowerShell is NOT called on the first-install path. + const powershellLog = path.join(fakeBin, "powershell.log") + expect(existsSync(powershellLog)).toBe(false) + }) +}) + +describe("install script — Windows locked binary with PowerShell available", () => { + let home: string + let fakeBin: string + + beforeAll(async () => { + home = await fs.mkdtemp(path.join(os.tmpdir(), "nikcli-home-win-locked-")) + trackTemp(home) + fakeBin = await buildFakeBin({ + unameS: "MINGW64_NT-10.0-19045", + unameM: "x86_64", + cygpathOutput: "C:\\fake\\translated", + powershell: true, + lockedMv: true, + }) + trackTemp(fakeBin) + + // Pre-populate the destination so install_binary takes the deferred path. + const installDir = path.join(home, ".nikcli", "bin") + await fs.mkdir(installDir, { recursive: true }) + const dest = path.join(installDir, "nikcli.exe") + await fs.writeFile(dest, SAMPLE_BINARY_V2, { mode: 0o755 }) + }) + + it("stages the new binary and defers finalization via powershell.exe", async () => { + const stagedBinary = path.join(fakeBin, "nikcli.exe.new") + await fs.writeFile(stagedBinary, SAMPLE_BINARY, { mode: 0o755 }) + + const { code, stderr } = await runInstall({ + home, + fakeBinDir: fakeBin, + args: ["--binary", stagedBinary], + }) + + expect(code).toBe(0) + expect(stderr).toContain("Update staged; restart nikcli to finish") + + // PowerShell shim was invoked exactly once with the helper file. + const powershellLog = path.join(fakeBin, "powershell.log") + expect(existsSync(powershellLog)).toBe(true) + const log = await fs.readFile(powershellLog, "utf8") + expect(log).toContain("fake-powershell called with") + expect(log).toContain("-File") + }) + + it("passes the cygpath-translated Windows path to powershell.exe", async () => { + // Reuse the same fakeBin — the powershell log is appended-to across + // tests in the suite. Reset it for this assertion. + const powershellLog = path.join(fakeBin, "powershell.log") + await fs.rm(powershellLog, { force: true }) + + const stagedBinary = path.join(fakeBin, "nikcli.exe.again") + await fs.writeFile(stagedBinary, SAMPLE_BINARY, { mode: 0o755 }) + + const { code } = await runInstall({ + home, + fakeBinDir: fakeBin, + args: ["--binary", stagedBinary], + }) + + expect(code).toBe(0) + const log = await fs.readFile(powershellLog, "utf8") + // Every translated path should be the cygpath output, never the raw + // POSIX /c/Users/... form. + expect(log).toContain("C:\\fake\\translated") + expect(log).not.toMatch(/\/c\/[A-Za-z]/) + }) +}) + +describe("install script — Windows locked binary without PowerShell", () => { + let home: string + let fakeBin: string + + beforeAll(async () => { + home = await fs.mkdtemp(path.join(os.tmpdir(), "nikcli-home-win-nops-")) + trackTemp(home) + fakeBin = await buildFakeBin({ + unameS: "MINGW64_NT-10.0-19045", + unameM: "x86_64", + cygpathOutput: "C:\\fake\\path", + // Note: NO powershell.exe in PATH, mv is locked + lockedMv: true, + }) + trackTemp(fakeBin) + + const installDir = path.join(home, ".nikcli", "bin") + await fs.mkdir(installDir, { recursive: true }) + const dest = path.join(installDir, "nikcli.exe") + await fs.writeFile(dest, SAMPLE_BINARY_V2, { mode: 0o755 }) + }) + + it("exits non-zero and does NOT leave a pending file behind", async () => { + const stagedBinary = path.join(fakeBin, "nikcli.exe.new") + await fs.writeFile(stagedBinary, SAMPLE_BINARY, { mode: 0o755 }) + + const { code, stderr } = await runInstall({ + home, + fakeBinDir: fakeBin, + args: ["--binary", stagedBinary], + }) + + // Without PowerShell, the installer must NOT claim success and must + // leave no .new. files on disk. + expect(code).not.toBe(0) + expect(stderr).toContain("powershell.exe is unavailable") + + const installDir = path.join(home, ".nikcli", "bin") + const files = await fs.readdir(installDir) + const stragglers = files.filter((f) => f.includes(".new.")) + expect(stragglers).toEqual([]) + }) +}) diff --git a/packages/nikcli/test/release/automation.test.ts b/packages/nikcli/test/release/automation.test.ts index d9ccd23de..dc391846b 100644 --- a/packages/nikcli/test/release/automation.test.ts +++ b/packages/nikcli/test/release/automation.test.ts @@ -99,7 +99,7 @@ describe("release automation", () => { it("installs Windows executables and defers replacement while nikcli.exe is running", async () => { const installer = await readRoot("install") - expect(installer).toContain('MINGW*|MSYS*|CYGWIN*) binary_name="$APP.exe"') + expect(installer).toContain('binary_name="$APP.exe"') expect(installer).toContain('local extracted_binary="$tmp_dir/bin/$binary_name"') expect(installer).toContain('extracted_binary="$tmp_dir/$ASSET_PREFIX-$target/bin/$binary_name"') expect(installer).toContain('local destination="$INSTALL_DIR/$binary_name"') @@ -109,6 +109,82 @@ describe("release automation", () => { expect(installer).toContain("install_deferred=true") }) + it("checks for powershell.exe before staging the deferred-replace binary", async () => { + const installer = await readRoot("install") + + // The require_powershell helper must be called before any pending file is + // written, otherwise the installer reports success while leaving a stale + // nikcli.exe.new.PID file behind on hosts without PowerShell. + const requireIndex = installer.indexOf("require_powershell()") + expect(requireIndex).toBeGreaterThan(-1) + const pendingIndex = installer.indexOf('local pending="${destination}.new.$$"') + expect(pendingIndex).toBeGreaterThan(requireIndex) + }) + + it("rejects PowerShell paths that cannot be translated to Windows form", async () => { + const installer = await readRoot("install") + + // windows_path must use cygpath when available and emit no trailing newline + // when falling back (PowerShell argument parsing tolerates a newline, but + // paths with embedded spaces become ambiguous when split on whitespace). + expect(installer).toMatch(/windows_path\(\)\s*\{[^}]*cygpath -w "\$1"/) + expect(installer).not.toMatch(/windows_path[^}]*printf "%s\\n" "\$1"/) + }) + + it("surfaces install_binary failures instead of masking them with deferred state", async () => { + const installer = await readRoot("install") + + // Both the direct-replace branch and the staging branch must check the + // return code of mv and abort with fail/exit 1 on failure. Without these + // checks a read-only install directory would print "Installed to ...". + expect(installer).toContain("Failed to install ${APP} to ${destination}") + expect(installer).toContain("Failed to stage pending ${APP} binary at ${pending}") + }) + + it("refuses to launch a 0-byte deferred-replace helper", async () => { + const installer = await readRoot("install") + + expect(installer).toContain('[ ! -s "$helper" ]') + expect(installer).toContain("is empty") + }) + + it("cleans the pending and helper files when the helper cannot be written", async () => { + const installer = await readRoot("install") + + // If the heredoc write fails, the installer must not leave the staged + // pending binary on disk: a future `nikcli` run would try to load it. + const writeFailureBlock = installer.match( + /then\s*\n\s*rm -f "\$pending"\s*\n\s*fail "Failed to write deferred-replace helper[^\n]*"/, + ) + expect(writeFailureBlock).not.toBeNull() + }) + + it("derives binary_name from os=windows, not from uname before platform detection", async () => { + const installer = await readRoot("install") + + // regression #6: binary_name was being set from `uname -s` before the + // platform-detection block had a chance to normalize `os`. The fix moves + // the assignment below the `os=...` derivation. + const osAssignment = installer.indexOf('os=$(echo "$raw_os"') + const binaryNameAssignment = installer.indexOf("binary_name=$APP") + expect(osAssignment).toBeGreaterThan(-1) + expect(binaryNameAssignment).toBeGreaterThan(osAssignment) + + // The assignment must be guarded by `os=windows`, not by an uname case + // statement that fires for any MSYS-style shell (e.g. on macOS hosts + // running inside an MSYS2 sub-shell). + expect(installer).toContain('if [ "$os" = "windows" ]; then\n binary_name="$APP.exe"') + }) + + it("uses the platform-correct binary name when reading from archives", async () => { + const installer = await readRoot("install") + + // After the regression #6 fix, the archive extraction must use + // `$binary_name` (which is APP.exe on Windows) instead of the bare `$APP`. + expect(installer).toContain('local extracted_binary="$tmp_dir/bin/$binary_name"') + expect(installer).not.toContain('local extracted_binary="$tmp_dir/bin/$APP"') + }) + it("does not bypass release safety checks or expose token output", async () => { const workflow = await readRoot(".github/workflows/publish.yml") const publishStart = await readRoot("script/publish-start.ts") From a13d2ba5f071ac7e2b6bb27cd48ff9a9a3d81bfb Mon Sep 17 00:00:00 2001 From: "nikcli-agent[bot]" Date: Tue, 16 Jun 2026 17:08:16 +0000 Subject: [PATCH 3/3] Hardened installer and added behavioral tests for PR #103. Co-authored-by: nikomatt69 --- .../.astro/content-assets.mjs | 2 +- .../.astro/content-modules.mjs | 2 +- .../inference-dashboard/.astro/content.d.ts | 374 ++++++++++-------- .../inference-dashboard/.astro/types.d.ts | 2 +- 4 files changed, 208 insertions(+), 172 deletions(-) diff --git a/packages/inference-dashboard/.astro/content-assets.mjs b/packages/inference-dashboard/.astro/content-assets.mjs index 11d135abe..2b8b8234b 100644 --- a/packages/inference-dashboard/.astro/content-assets.mjs +++ b/packages/inference-dashboard/.astro/content-assets.mjs @@ -1 +1 @@ -export default new Map() +export default new Map(); \ No newline at end of file diff --git a/packages/inference-dashboard/.astro/content-modules.mjs b/packages/inference-dashboard/.astro/content-modules.mjs index 11d135abe..2b8b8234b 100644 --- a/packages/inference-dashboard/.astro/content-modules.mjs +++ b/packages/inference-dashboard/.astro/content-modules.mjs @@ -1 +1 @@ -export default new Map() +export default new Map(); \ No newline at end of file diff --git a/packages/inference-dashboard/.astro/content.d.ts b/packages/inference-dashboard/.astro/content.d.ts index 8edb6f3ad..0a87baa34 100644 --- a/packages/inference-dashboard/.astro/content.d.ts +++ b/packages/inference-dashboard/.astro/content.d.ts @@ -1,174 +1,210 @@ -declare module "astro:content" { - interface Render { - ".mdx": Promise<{ - Content: import("astro").MDXContent - headings: import("astro").MarkdownHeading[] - remarkPluginFrontmatter: Record - components: import("astro").MDXInstance<{}>["components"] - }> - } +declare module 'astro:content' { + interface Render { + '.mdx': Promise<{ + Content: import('astro').MDXContent; + headings: import('astro').MarkdownHeading[]; + remarkPluginFrontmatter: Record; + components: import('astro').MDXInstance<{}>['components']; + }>; + } } -declare module "astro:content" { - export interface RenderResult { - Content: import("astro/runtime/server/index.js").AstroComponentFactory - headings: import("astro").MarkdownHeading[] - remarkPluginFrontmatter: Record - } - interface Render { - ".md": Promise - } - - export interface RenderedContent { - html: string - metadata?: { - imagePaths: Array - [key: string]: unknown - } - } +declare module 'astro:content' { + export interface RenderResult { + Content: import('astro/runtime/server/index.js').AstroComponentFactory; + headings: import('astro').MarkdownHeading[]; + remarkPluginFrontmatter: Record; + } + interface Render { + '.md': Promise; + } + + export interface RenderedContent { + html: string; + metadata?: { + imagePaths: Array; + [key: string]: unknown; + }; + } } -declare module "astro:content" { - type Flatten = T extends { [K: string]: infer U } ? U : never - - export type CollectionKey = keyof AnyEntryMap - export type CollectionEntry = Flatten - - export type ContentCollectionKey = keyof ContentEntryMap - export type DataCollectionKey = keyof DataEntryMap - - type AllValuesOf = T extends any ? T[keyof T] : never - type ValidContentEntrySlug = AllValuesOf["slug"] - - export type ReferenceDataEntry = { - collection: C - id: E - } - export type ReferenceContentEntry< - C extends keyof ContentEntryMap, - E extends ValidContentEntrySlug | (string & {}) = string, - > = { - collection: C - slug: E - } - export type ReferenceLiveEntry = { - collection: C - id: string - } - - /** @deprecated Use `getEntry` instead. */ - export function getEntryBySlug | (string & {})>( - collection: C, - // Note that this has to accept a regular string too, for SSR - entrySlug: E, - ): E extends ValidContentEntrySlug ? Promise> : Promise | undefined> - - /** @deprecated Use `getEntry` instead. */ - export function getDataEntryById( - collection: C, - entryId: E, - ): Promise> - - export function getCollection>( - collection: C, - filter?: (entry: CollectionEntry) => entry is E, - ): Promise - export function getCollection( - collection: C, - filter?: (entry: CollectionEntry) => unknown, - ): Promise[]> - - export function getLiveCollection( - collection: C, - filter?: LiveLoaderCollectionFilterType, - ): Promise, LiveLoaderErrorType>> - - export function getEntry | (string & {})>( - entry: ReferenceContentEntry, - ): E extends ValidContentEntrySlug ? Promise> : Promise | undefined> - export function getEntry( - entry: ReferenceDataEntry, - ): E extends keyof DataEntryMap[C] ? Promise : Promise | undefined> - export function getEntry | (string & {})>( - collection: C, - slug: E, - ): E extends ValidContentEntrySlug ? Promise> : Promise | undefined> - export function getEntry( - collection: C, - id: E, - ): E extends keyof DataEntryMap[C] - ? string extends keyof DataEntryMap[C] - ? Promise | undefined - : Promise - : Promise | undefined> - export function getLiveEntry( - collection: C, - filter: string | LiveLoaderEntryFilterType, - ): Promise, LiveLoaderErrorType>> - - /** Resolve an array of entry references from the same collection */ - export function getEntries( - entries: ReferenceContentEntry>[], - ): Promise[]> - export function getEntries( - entries: ReferenceDataEntry[], - ): Promise[]> - - export function render(entry: AnyEntryMap[C][string]): Promise - - export function reference( - collection: C, - ): import("astro/zod").ZodEffects< - import("astro/zod").ZodString, - C extends keyof ContentEntryMap - ? ReferenceContentEntry> - : ReferenceDataEntry - > - // Allow generic `string` to avoid excessive type errors in the config - // if `dev` is not running to update as you edit. - // Invalid collection names will be caught at build time. - export function reference( - collection: C, - ): import("astro/zod").ZodEffects - - type ReturnTypeOrOriginal = T extends (...args: any[]) => infer R ? R : T - type InferEntrySchema = import("astro/zod").infer< - ReturnTypeOrOriginal["schema"]> - > - - type ContentEntryMap = {} - - type DataEntryMap = {} - - type AnyEntryMap = ContentEntryMap & DataEntryMap - - type ExtractLoaderTypes = T extends import("astro/loaders").LiveLoader< - infer TData, - infer TEntryFilter, - infer TCollectionFilter, - infer TError - > - ? { data: TData; entryFilter: TEntryFilter; collectionFilter: TCollectionFilter; error: TError } - : { data: never; entryFilter: never; collectionFilter: never; error: never } - type ExtractDataType = ExtractLoaderTypes["data"] - type ExtractEntryFilterType = ExtractLoaderTypes["entryFilter"] - type ExtractCollectionFilterType = ExtractLoaderTypes["collectionFilter"] - type ExtractErrorType = ExtractLoaderTypes["error"] - - type LiveLoaderDataType = - LiveContentConfig["collections"][C]["schema"] extends undefined - ? ExtractDataType - : import("astro/zod").infer> - type LiveLoaderEntryFilterType = ExtractEntryFilterType< - LiveContentConfig["collections"][C]["loader"] - > - type LiveLoaderCollectionFilterType = ExtractCollectionFilterType< - LiveContentConfig["collections"][C]["loader"] - > - type LiveLoaderErrorType = ExtractErrorType< - LiveContentConfig["collections"][C]["loader"] - > - - export type ContentConfig = typeof import("../src/content.config.mjs") - export type LiveContentConfig = never +declare module 'astro:content' { + type Flatten = T extends { [K: string]: infer U } ? U : never; + + export type CollectionKey = keyof AnyEntryMap; + export type CollectionEntry = Flatten; + + export type ContentCollectionKey = keyof ContentEntryMap; + export type DataCollectionKey = keyof DataEntryMap; + + type AllValuesOf = T extends any ? T[keyof T] : never; + type ValidContentEntrySlug = AllValuesOf< + ContentEntryMap[C] + >['slug']; + + export type ReferenceDataEntry< + C extends CollectionKey, + E extends keyof DataEntryMap[C] = string, + > = { + collection: C; + id: E; + }; + export type ReferenceContentEntry< + C extends keyof ContentEntryMap, + E extends ValidContentEntrySlug | (string & {}) = string, + > = { + collection: C; + slug: E; + }; + export type ReferenceLiveEntry = { + collection: C; + id: string; + }; + + /** @deprecated Use `getEntry` instead. */ + export function getEntryBySlug< + C extends keyof ContentEntryMap, + E extends ValidContentEntrySlug | (string & {}), + >( + collection: C, + // Note that this has to accept a regular string too, for SSR + entrySlug: E, + ): E extends ValidContentEntrySlug + ? Promise> + : Promise | undefined>; + + /** @deprecated Use `getEntry` instead. */ + export function getDataEntryById( + collection: C, + entryId: E, + ): Promise>; + + export function getCollection>( + collection: C, + filter?: (entry: CollectionEntry) => entry is E, + ): Promise; + export function getCollection( + collection: C, + filter?: (entry: CollectionEntry) => unknown, + ): Promise[]>; + + export function getLiveCollection( + collection: C, + filter?: LiveLoaderCollectionFilterType, + ): Promise< + import('astro').LiveDataCollectionResult, LiveLoaderErrorType> + >; + + export function getEntry< + C extends keyof ContentEntryMap, + E extends ValidContentEntrySlug | (string & {}), + >( + entry: ReferenceContentEntry, + ): E extends ValidContentEntrySlug + ? Promise> + : Promise | undefined>; + export function getEntry< + C extends keyof DataEntryMap, + E extends keyof DataEntryMap[C] | (string & {}), + >( + entry: ReferenceDataEntry, + ): E extends keyof DataEntryMap[C] + ? Promise + : Promise | undefined>; + export function getEntry< + C extends keyof ContentEntryMap, + E extends ValidContentEntrySlug | (string & {}), + >( + collection: C, + slug: E, + ): E extends ValidContentEntrySlug + ? Promise> + : Promise | undefined>; + export function getEntry< + C extends keyof DataEntryMap, + E extends keyof DataEntryMap[C] | (string & {}), + >( + collection: C, + id: E, + ): E extends keyof DataEntryMap[C] + ? string extends keyof DataEntryMap[C] + ? Promise | undefined + : Promise + : Promise | undefined>; + export function getLiveEntry( + collection: C, + filter: string | LiveLoaderEntryFilterType, + ): Promise, LiveLoaderErrorType>>; + + /** Resolve an array of entry references from the same collection */ + export function getEntries( + entries: ReferenceContentEntry>[], + ): Promise[]>; + export function getEntries( + entries: ReferenceDataEntry[], + ): Promise[]>; + + export function render( + entry: AnyEntryMap[C][string], + ): Promise; + + export function reference( + collection: C, + ): import('astro/zod').ZodEffects< + import('astro/zod').ZodString, + C extends keyof ContentEntryMap + ? ReferenceContentEntry> + : ReferenceDataEntry + >; + // Allow generic `string` to avoid excessive type errors in the config + // if `dev` is not running to update as you edit. + // Invalid collection names will be caught at build time. + export function reference( + collection: C, + ): import('astro/zod').ZodEffects; + + type ReturnTypeOrOriginal = T extends (...args: any[]) => infer R ? R : T; + type InferEntrySchema = import('astro/zod').infer< + ReturnTypeOrOriginal['schema']> + >; + + type ContentEntryMap = { + + }; + + type DataEntryMap = { + + }; + + type AnyEntryMap = ContentEntryMap & DataEntryMap; + + type ExtractLoaderTypes = T extends import('astro/loaders').LiveLoader< + infer TData, + infer TEntryFilter, + infer TCollectionFilter, + infer TError + > + ? { data: TData; entryFilter: TEntryFilter; collectionFilter: TCollectionFilter; error: TError } + : { data: never; entryFilter: never; collectionFilter: never; error: never }; + type ExtractDataType = ExtractLoaderTypes['data']; + type ExtractEntryFilterType = ExtractLoaderTypes['entryFilter']; + type ExtractCollectionFilterType = ExtractLoaderTypes['collectionFilter']; + type ExtractErrorType = ExtractLoaderTypes['error']; + + type LiveLoaderDataType = + LiveContentConfig['collections'][C]['schema'] extends undefined + ? ExtractDataType + : import('astro/zod').infer< + Exclude + >; + type LiveLoaderEntryFilterType = + ExtractEntryFilterType; + type LiveLoaderCollectionFilterType = + ExtractCollectionFilterType; + type LiveLoaderErrorType = ExtractErrorType< + LiveContentConfig['collections'][C]['loader'] + >; + + export type ContentConfig = typeof import("../src/content.config.mjs"); + export type LiveContentConfig = never; } diff --git a/packages/inference-dashboard/.astro/types.d.ts b/packages/inference-dashboard/.astro/types.d.ts index 306a68bfd..03d7cc43f 100644 --- a/packages/inference-dashboard/.astro/types.d.ts +++ b/packages/inference-dashboard/.astro/types.d.ts @@ -1,2 +1,2 @@ /// -/// +/// \ No newline at end of file