From 570b1daafcd84931fd1a0f755535feb3fa0425ef Mon Sep 17 00:00:00 2001 From: xinoai <107881109+xinoai@users.noreply.github.com> Date: Wed, 2 Sep 2026 13:20:40 +0000 Subject: [PATCH] fix(cli): keep pre-commit hooks on external installations --- sdk/typescript/README.md | 8 +- sdk/typescript/src/cli.ts | 61 ++++++++-- sdk/typescript/tests-ts/cli-launcher.test.ts | 47 +++++++ sdk/typescript/tests-ts/cli.test.ts | 121 +++++++++++++++++++ 4 files changed, 224 insertions(+), 13 deletions(-) diff --git a/sdk/typescript/README.md b/sdk/typescript/README.md index d3be46141..03911fc8d 100644 --- a/sdk/typescript/README.md +++ b/sdk/typescript/README.md @@ -893,7 +893,13 @@ reject `--json`. `install-hook` scans staged and unstaged changes before each commit. It blocks on high-severity findings or failed scans, respects `core.hooksPath`, and leaves -existing hooks alone. Change the threshold with `--fail-on-severity`. +custom hooks alone. Change the threshold with `--fail-on-severity`. +The Codex Security installation and Node runtime must be outside the scanned +repository. Install the CLI globally, then run `codex-security install-hook` +from the repository. Repository-local installations cannot install a hook. +To refresh an existing generated hook, rerun `install-hook` from the external +installation with the same severity threshold. Updating the package alone does +not update installed hooks. ### Import alerts from the CLI diff --git a/sdk/typescript/src/cli.ts b/sdk/typescript/src/cli.ts index d803ed425..d4bba7332 100644 --- a/sdk/typescript/src/cli.ts +++ b/sdk/typescript/src/cli.ts @@ -3078,7 +3078,8 @@ export async function main( }, }) .command("install-hook", { - description: "Install a Git pre-commit security scan.", + description: + "Install a Git pre-commit security scan from an installation outside the repository.", destructive: true, mcp: false, args: z.object({ @@ -3101,14 +3102,38 @@ export async function main( .optional(), async run({ args, options }) { try { + const repository = resolveCliPath( + dependencies.currentDirectory(), + args.repository ?? ".", + ); + const worktree = await realpath( + execFileSync( + "git", + ["-C", repository, "rev-parse", "--show-toplevel"], + { + encoding: "utf8", + }, + ).trim(), + ); + const runtime = realpathSync(process.execPath); + const cli = realpathSync(fileURLToPath(import.meta.url)); + const installation = realpathSync( + fileURLToPath(new URL("..", import.meta.url)), + ); + if ( + [runtime, cli, installation].some( + (path) => !isOutsidePath(relative(worktree, path)), + ) + ) { + throw new Error( + "Install the pre-commit hook using a Codex Security installation and Node runtime outside this repository, such as a global npm installation.", + ); + } const hook = execFileSync( "git", [ "-C", - resolveCliPath( - dependencies.currentDirectory(), - args.repository ?? ".", - ), + repository, "rev-parse", "--path-format=absolute", "--git-path", @@ -3116,26 +3141,38 @@ export async function main( ], { encoding: "utf8" }, ).trim(); - const command = [ - realpathSync(process.execPath), - realpathSync(fileURLToPath(import.meta.url)), - ] + const command = [runtime, cli] .map((path) => `'${path.replaceAll("'", `'"'"'`)}'`) .join(" "); - const contents = `#!/bin/sh\nset -eu\nexec ${command} scan . --working-tree --fail-on-severity ${options.failOnSeverity}\n`; + const contents = `#!/bin/sh\n# Managed by Codex Security.\nset -eu\nexec ${command} scan . --working-tree --fail-on-severity ${options.failOnSeverity}\n`; const legacyContents = `#!/bin/sh\nset -eu\nexec npx --no-install codex-security scan . --working-tree --fail-on-severity ${options.failOnSeverity}\n`; const existing = await readFile(hook, "utf8").catch(() => null); + const quotedPath = "'(?:[^']|'\"'\"')*'"; + const pinnedHook = existing?.match( + new RegExp( + `^#!/bin/sh\\n(?:# Managed by Codex Security\\.\\n)?set -eu\\nexec (${quotedPath}) (${quotedPath}) scan \\. --working-tree --fail-on-severity ${options.failOnSeverity}\\n$`, + "u", + ), + ); + const generatedPinnedHook = + pinnedHook != null && + pinnedHook + .slice(1) + .every((path) => + isAbsolute(path.slice(1, -1).replaceAll(`'"'"'`, "'")), + ); if ( existing !== null && existing !== contents && - existing !== legacyContents + existing !== legacyContents && + !generatedPinnedHook ) { throw new Error(`A pre-commit hook already exists at ${hook}.`); } if (existing === null) { await mkdir(dirname(hook), { recursive: true }); await writeFile(hook, contents, { flag: "wx", mode: 0o755 }); - } else if (existing === legacyContents) { + } else if (existing !== contents) { await writeFile(hook, contents, { flag: "w" }); } return { diff --git a/sdk/typescript/tests-ts/cli-launcher.test.ts b/sdk/typescript/tests-ts/cli-launcher.test.ts index 35acc1c53..82932bbeb 100644 --- a/sdk/typescript/tests-ts/cli-launcher.test.ts +++ b/sdk/typescript/tests-ts/cli-launcher.test.ts @@ -18,6 +18,33 @@ import { runCommand } from "./support/shell.js"; const packageRoot = join(import.meta.dir, ".."); describe("CLI launcher", () => { + test("requires a runtime outside the target worktree when installing a hook", async () => { + const root = await mkdtemp(join(tmpdir(), "codex-security-hook-runtime-")); + try { + const initialized = await runCommand("git", ["init", "-q", root], { + timeout: 10_000, + }); + expect(initialized.status, initialized.stderr).toBe(0); + const runtime = join( + root, + process.platform === "win32" ? "bun.exe" : "bun", + ); + await copyFile(process.execPath, runtime); + const result = await runCommand( + runtime, + [join(packageRoot, "src", "cli.ts"), "install-hook", root], + { timeout: 30_000 }, + ); + expect(result.status, result.stderr).toBe(2); + expect(result.stderr).toContain("outside this repository"); + await expect( + readFile(join(root, ".git", "hooks", "pre-commit")), + ).rejects.toMatchObject({ code: "ENOENT" }); + } finally { + await rm(root, { recursive: true, force: true }); + } + }); + test("runs through an installed npm-style bin symlink", async () => { const root = await mkdtemp(join(tmpdir(), "codex-security-cli-bin-")); try { @@ -154,6 +181,26 @@ describe("CLI launcher", () => { expect(child.stderr).toBe(""); expect(child.stdout).toBe(`${VERSION}\n`); + const initialized = await runCommand("git", ["init", "-q", root], { + timeout: 10_000, + }); + expect(initialized.status, initialized.stderr).toBe(0); + const subdirectory = join(root, "source"); + await mkdir(subdirectory); + const install = await runCommand( + "node", + [bin, "install-hook", subdirectory], + { + env: launchEnvironment, + timeout: 30_000, + }, + ); + expect(install.status, install.stderr).toBe(2); + expect(install.stderr).toContain("outside this repository"); + await expect( + readFile(join(root, ".git", "hooks", "pre-commit")), + ).rejects.toMatchObject({ code: "ENOENT" }); + const preload = join(root, "unavailable-cwd.mjs"); await writeFile( preload, diff --git a/sdk/typescript/tests-ts/cli.test.ts b/sdk/typescript/tests-ts/cli.test.ts index 897ca5041..0f6c3f873 100644 --- a/sdk/typescript/tests-ts/cli.test.ts +++ b/sdk/typescript/tests-ts/cli.test.ts @@ -748,6 +748,127 @@ describe("CLI", () => { } }); + test("refreshes a generated hook before replacing a local installation on checkout", async () => { + const root = await realpath( + await mkdtemp(join(tmpdir(), "codex-security-hook-refresh-")), + ); + try { + const repository = join(root, "worktree's files"); + await mkdir(repository); + const git = async (args: string[]) => { + const result = await runCommand( + "git", + [ + "-C", + repository, + "-c", + "user.name=Test", + "-c", + "user.email=test@example.com", + "-c", + "commit.gpgsign=false", + ...args, + ], + { timeout: 10_000 }, + ); + expect(result.status, result.stderr).toBe(0); + return result.stdout.trim(); + }; + await git(["init", "-q", "-b", "original"]); + await writeFile(join(repository, ".gitignore"), "node_modules/\n"); + await git(["add", ".gitignore"]); + await git(["commit", "--no-verify", "-qm", "initial"]); + + const localCli = join( + repository, + "node_modules", + "@openai", + "codex-security", + "dist", + "cli.js", + ); + await mkdir(join(localCli, ".."), { recursive: true }); + const marker = join(root, "local-module-used"); + await writeFile( + localCli, + `import { writeFileSync } from "node:fs";\nwriteFileSync(${JSON.stringify(marker)}, "used");\n`, + ); + await git(["switch", "-qc", "replacement"]); + await git([ + "add", + "-f", + "node_modules/@openai/codex-security/dist/cli.js", + ]); + await git(["commit", "--no-verify", "-qm", "fixture update"]); + await git(["switch", "-q", "original"]); + await mkdir(join(localCli, ".."), { recursive: true }); + await writeFile(localCli, "throw new Error('old installation');\n"); + + const quote = (path: string) => `'${path.replaceAll("'", `'"'"'`)}'`; + const hook = join(repository, ".git", "hooks", "pre-commit"); + await writeFile( + hook, + `#!/bin/sh\nset -eu\nexec ${quote(await realpath(process.execPath))} ${quote(localCli)} scan . --working-tree --fail-on-severity high\n`, + { mode: 0o755 }, + ); + const stderr = capture(); + expect( + await main( + ["install-hook", repository], + capture().stream, + stderr.stream, + dependencies(), + ), + ).toBe(0); + expect(stderr.text()).toBe(""); + const refreshed = await readFile(hook, "utf8"); + expect(refreshed).toContain("# Managed by Codex Security."); + expect(refreshed).not.toContain(quote(localCli)); + expect(refreshed).toContain( + quote( + await realpath( + fileURLToPath(new URL("../src/cli.ts", import.meta.url)), + ), + ), + ); + + await git(["switch", "-q", "replacement"]); + expect(await readFile(localCli, "utf8")).toContain("writeFileSync"); + const before = await git(["rev-parse", "HEAD"]); + const commit = await runCommand( + "git", + [ + "-C", + repository, + "-c", + "user.name=Test", + "-c", + "user.email=test@example.com", + "-c", + "commit.gpgsign=false", + "commit", + "--allow-empty", + "-qm", + "fixture commit", + ], + { + env: { + ...process.env, + CODEX_HOME: join(root, "codex-home"), + CODEX_API_KEY: "", + OPENAI_API_KEY: "", + }, + timeout: 10_000, + }, + ); + expect(commit.status, commit.stderr).toBeGreaterThan(0); + await expect(readFile(marker)).rejects.toMatchObject({ code: "ENOENT" }); + expect(await git(["rev-parse", "HEAD"])).toBe(before); + } finally { + await rm(root, { recursive: true, force: true }); + } + }); + test("runs a bulk scan and keeps structured output on stdout", async () => { const root = await mkdtemp(join(tmpdir(), "codex-security-cli-multiscan-")); try {