From 871dbe0302ebbe331a87c513e219f04bed34ec98 Mon Sep 17 00:00:00 2001 From: Dotta Date: Wed, 22 Jul 2026 22:26:48 +0000 Subject: [PATCH] fix(cli): secure git installs and restart locks Co-Authored-By: Paperclip --- cli/src/__tests__/install-command.test.ts | 13 +++++++++-- .../__tests__/service-health-check.test.ts | 16 +++++++++++++ cli/src/commands/install.ts | 18 ++++++++++++++- cli/src/commands/service.ts | 23 ++++++++++++++++++- cli/src/index.ts | 2 +- 5 files changed, 67 insertions(+), 5 deletions(-) diff --git a/cli/src/__tests__/install-command.test.ts b/cli/src/__tests__/install-command.test.ts index 6dfe6eba20a..c2a45f3b59f 100644 --- a/cli/src/__tests__/install-command.test.ts +++ b/cli/src/__tests__/install-command.test.ts @@ -73,6 +73,15 @@ describe("managed install commands", () => { expect(() => resolveGitInstallRequest({ repo: "HenkDz/paperclip" })).toThrow("requires --ref"); }); + it("requires explicit non-interactive consent before resolving git refs", async () => { + const runCommand = vi.fn(); + + await expect(installCommand({ ref: "master", repo: "HenkDz/paperclip" }, { runCommand })) + .rejects.toThrow("Re-run with --yes"); + + expect(runCommand).not.toHaveBeenCalled(); + }); + it("reuses a SHA-keyed git payload without downloading or rebuilding", async () => { const sha = "b".repeat(40); const paths = resolveInstallStorePaths(); @@ -107,8 +116,8 @@ describe("managed install commands", () => { if (file === process.execPath) return { stdout: "0.3.1\n", stderr: "" }; throw new Error(`Unexpected command: ${file} ${args.join(" ")}`); }); - await installCommand({ ref: "master", repo: "HenkDz/paperclip" }, { runCommand }); - await installCommand({ ref: "master", repo: "HenkDz/paperclip" }, { runCommand }); + await installCommand({ ref: "master", repo: "HenkDz/paperclip", yes: true }, { runCommand }); + await installCommand({ ref: "master", repo: "HenkDz/paperclip", yes: true }, { runCommand }); const manifest = readInstallManifest(resolveInstallStorePaths()); expect(manifest).toMatchObject({ source: "git", repo: "HenkDz/paperclip", ref: "master", sha }); expect(manifest?.payloadPath).toContain(path.join("git", sha.slice(0, 12))); diff --git a/cli/src/__tests__/service-health-check.test.ts b/cli/src/__tests__/service-health-check.test.ts index b6a9c4cd307..d913b2771da 100644 --- a/cli/src/__tests__/service-health-check.test.ts +++ b/cli/src/__tests__/service-health-check.test.ts @@ -93,6 +93,22 @@ describe("service health doctor checks", () => { expect(order).toEqual(["first-start", "first-end", "second-start"]); }); + it("reclaims restart locks left by terminated processes", async () => { + const lockPath = path.join(process.env.PAPERCLIP_HOME!, "instances", "default", "hot-restart.lock"); + fs.mkdirSync(path.dirname(lockPath), { recursive: true }); + fs.writeFileSync(lockPath, "424242:stale-token\n"); + const callback = vi.fn(async () => "restarted"); + + await expect(withHotRestartLock("default", callback, { + pollMs: 1, + timeoutMs: 20, + isProcessAlive: () => false, + })).resolves.toBe("restarted"); + + expect(callback).toHaveBeenCalledOnce(); + expect(fs.existsSync(lockPath)).toBe(false); + }); + it("brackets configured IPv6 hosts in health URLs", () => { expect(buildLocalHealthUrl("::1", 3100)).toBe("http://[::1]:3100/api/health"); expect(buildLocalHealthUrl("::", 3100)).toBe("http://127.0.0.1:3100/api/health"); diff --git a/cli/src/commands/install.ts b/cli/src/commands/install.ts index d1dd2c9f510..0da2855e573 100644 --- a/cli/src/commands/install.ts +++ b/cli/src/commands/install.ts @@ -236,6 +236,22 @@ async function ensureShimOnPath(options: InstallOptions): Promise { console.log(changed ? pc.green(`Updated ${rcPath}.`) : pc.dim(`${rcPath} already contains the PATH block.`)); } +async function confirmGitInstall(options: InstallOptions, repo: string, ref: string): Promise { + const warning = `Installing ${repo}@${ref} executes dependency and build scripts from that repository.`; + console.log(pc.yellow(`Warning: ${warning}`)); + if (options.yes === true) return; + if (!process.stdin.isTTY || !process.stdout.isTTY) { + throw new Error(`${warning} Re-run with --yes to consent in non-interactive environments.`); + } + const confirmed = await p.confirm({ + message: `${warning} Continue?`, + initialValue: false, + }); + if (p.isCancel(confirmed) || !confirmed) { + throw new Error("Git-ref install cancelled before downloading or executing repository code."); + } +} + export async function installCommand( options: InstallOptions, dependencies: { runCommand?: CommandRunner; now?: () => Date } = {}, @@ -244,6 +260,7 @@ export async function installCommand( const runCommand = dependencies.runCommand ?? execFileAsync; const gitRequest = resolveGitInstallRequest(options); if (gitRequest) { + await confirmGitInstall(options, gitRequest.repo, gitRequest.ref); const sha = await resolveGitHubRef(gitRequest.repo, gitRequest.ref, runCommand); const paths = resolveInstallStorePaths(); const installed = await withInstallStoreLock(async () => { @@ -258,7 +275,6 @@ export async function installCommand( writeManagedShim(paths); pruneInstallPayloads(nextManifest, paths); return payload; }, paths); await ensureShimOnPath(options); - console.log(pc.yellow(`Warning: running unreleased code from ${gitRequest.repo}@${gitRequest.ref} (${sha.slice(0, 12)}).`)); console.log(pc.green(`${installed.reused ? "Activated cached" : "Installed"} paperclipai git payload ${sha.slice(0, 12)}.`)); return; } diff --git a/cli/src/commands/service.ts b/cli/src/commands/service.ts index d5142bcf7e9..8e8a689dc6a 100644 --- a/cli/src/commands/service.ts +++ b/cli/src/commands/service.ts @@ -57,13 +57,21 @@ export function resolveRestartExpectedVersion(expectedVersion: string | null | u export async function withHotRestartLock( instanceId: string, callback: () => Promise, - options: { timeoutMs?: number; pollMs?: number } = {}, + options: { timeoutMs?: number; pollMs?: number; isProcessAlive?: (pid: number) => boolean } = {}, ): Promise { const instanceRoot = resolvePaperclipInstanceRoot(instanceId); const lockPath = path.join(instanceRoot, "hot-restart.lock"); const token = `${process.pid}:${Date.now()}:${Math.random().toString(16).slice(2)}`; const deadline = Date.now() + (options.timeoutMs ?? 120_000); const pollMs = options.pollMs ?? 100; + const isProcessAlive = options.isProcessAlive ?? ((pid: number) => { + try { + process.kill(pid, 0); + return true; + } catch (error) { + return (error as NodeJS.ErrnoException).code !== "ESRCH"; + } + }); await fs.mkdir(instanceRoot, { recursive: true }); while (true) { @@ -72,6 +80,19 @@ export async function withHotRestartLock( break; } catch (error) { if ((error as NodeJS.ErrnoException).code !== "EEXIST") throw error; + try { + const existingToken = (await fs.readFile(lockPath, "utf8")).trim(); + const ownerPid = Number.parseInt(existingToken.split(":", 1)[0] ?? "", 10); + if (Number.isInteger(ownerPid) && ownerPid > 0 && !isProcessAlive(ownerPid)) { + if ((await fs.readFile(lockPath, "utf8")).trim() === existingToken) { + await fs.rm(lockPath, { force: true }); + continue; + } + } + } catch (readError) { + if ((readError as NodeJS.ErrnoException).code === "ENOENT") continue; + throw readError; + } if (Date.now() >= deadline) { throw new Error( `Another restart for instance ${instanceId} is still running. ` + diff --git a/cli/src/index.ts b/cli/src/index.ts index ac4c74e27c5..53598b04391 100644 --- a/cli/src/index.ts +++ b/cli/src/index.ts @@ -66,7 +66,7 @@ program .option("--version ", "Install an exact published npm version") .option("--ref ", "Install a GitHub branch, tag, or commit SHA") .option("--repo ", "Override the GitHub repository used with --ref") - .option("-y, --yes", "Consent to the supported shell PATH update without prompting") + .option("-y, --yes", "Consent to git-ref code execution and supported shell PATH updates without prompting") .action(installCommand); program