From 61d668af065523e9f6dabc1ec5e1adccaad35d85 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 20:02:35 +0000 Subject: [PATCH 01/19] fix(hooks): give nobody test runs the .nvmrc node and drop an unreadable CA bundle In Claude Code's cloud containers the setpriv prefix ran the container's own node, another major version than .nvmrc, and inherited a NODE_EXTRA_CA_CERTS file nobody cannot read. The first fails every test that needs a newer node API; the second adds a warning to every child process's stderr. - When nobody's node is another major version but the session's matches .nvmrc, the hook copies the session's node into nobody's HOME and puts it first on PATH in the prefix. - When nobody cannot read the NODE_EXTRA_CA_CERTS file, the prefix unsets it. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01PkA1FHrosoEim4HpZyQGyQ --- .claude/hooks/require-nonroot-tests.sh | 77 +++++++++-- src/__tests__/require-nonroot-tests.test.ts | 140 +++++++++++++++++++- 2 files changed, 199 insertions(+), 18 deletions(-) diff --git a/.claude/hooks/require-nonroot-tests.sh b/.claude/hooks/require-nonroot-tests.sh index 3d2f925b8..71c608d06 100755 --- a/.claude/hooks/require-nonroot-tests.sh +++ b/.claude/hooks/require-nonroot-tests.sh @@ -7,6 +7,9 @@ # exits at its first check, so it costs nothing on a developer machine. # - Before replying, it makes the folders a test run writes in the checkout # writable for nobody. +# - The prefix it offers matches CI where it can: it runs the .nvmrc Node +# when nobody's own node is another version, and it drops a +# NODE_EXTRA_CA_CERTS file nobody cannot read. # - .claude/settings.json registers the hook on PreToolUse for Bash with a # 10-second timeout. Claude Code cancels a slower hook and runs the command # anyway, so the hook must stay fast. @@ -288,6 +291,16 @@ if [[ -z "${nobody_node_version}" ]]; then exit 0 fi +# The major versions are compared only when .nvmrc names a number (24 or +# v24.1.0), not an alias such as lts/*. Only the file's first word counts, so +# a CRLF line end or a comment after the version does not hide it. +nvmrc_version="$(cat "${checkout}/.nvmrc" 2>/dev/null)" || nvmrc_version="" +nvmrc_version="${nvmrc_version%%[[:space:]]*}" +nvmrc_major="${nvmrc_version#v}" +nvmrc_major="${nvmrc_major%%.*}" +nobody_node_major="${nobody_node_version#v}" +nobody_node_major="${nobody_node_major%%.*}" + # A test run writes in these folders: vitest's caches under node_modules, a # fake HOME that keeps npm's cache and logs out of root's home, and coverage/ # for test:coverage's report (gitignored). @@ -299,6 +312,31 @@ nobody_writable=( ) mkdir -p "${nobody_writable[@]}" +# When nobody's node is another major version than .nvmrc names but the +# session's own node matches it, the prefix runs a copy of the session's node. +# nobody cannot run the original, because nvm installs it under root's home. +# - The copy sits in nobody's HOME, named by version, so a later run reuses it +# and a different version gets its own folder. +# - cp -L copies the binary a symlink such as nvm's points to. +# - The prefix puts the copy's folder first on PATH, so the npx and tsx +# processes a run starts use it too. +nobody_node_dir="" + +if [[ "${nvmrc_major}" =~ ^[0-9]+$ && "${nobody_node_major}" != "${nvmrc_major}" ]]; then + session_node_version="$(node --version 2>/dev/null)" || session_node_version="" + session_node_major="${session_node_version#v}" + session_node_major="${session_node_major%%.*}" + + if [[ "${session_node_major}" == "${nvmrc_major}" ]]; then + nobody_node_dir="${checkout}/node_modules/.nobody-home/node-${session_node_version}" + + if [[ ! -x "${nobody_node_dir}/node" ]]; then + mkdir -p "${nobody_node_dir}" + cp -L "$(command -v node)" "${nobody_node_dir}/node" + fi + fi +fi + # snapshot:update rewrites the tool-surface baseline in this folder, the only # snapshot folder in the repo. git tracks neither owner nor write bits, so the # changes below add nothing to a diff. @@ -319,24 +357,39 @@ chmod -R go-w "${nobody_writable[@]}" # checkout's path holds a space. A path of plain characters prints unchanged. printf -v nobody_home_for_shell '%q' "${checkout}/node_modules/.nobody-home" +# env's settings, each followed by a space. $PATH stays literal, so the shell +# running the prefix expands it. +env_settings="" + +# Claude Code's cloud containers point NODE_EXTRA_CA_CERTS at a file under +# root's home. Every node nobody starts then prints a warning that it cannot +# read the file, which breaks tests that check a child process's exact stderr. +# The tests make no network calls, so the prefix drops the setting. /bin/sh is +# named in full because nobody's PATH may not hold sh, and its $1 is +# single-quoted on purpose: the inner shell expands it, not this one. +# shellcheck disable=SC2016 +if [[ -n "${NODE_EXTRA_CA_CERTS:-}" ]] && + ! setpriv --reuid="${nobody_uid}" --regid="${nobody_gid}" --clear-groups /bin/sh -c 'test -r "$1"' sh "${NODE_EXTRA_CA_CERTS}"; then + env_settings+="-u NODE_EXTRA_CA_CERTS " +fi + +env_settings+="HOME=${nobody_home_for_shell}" + +if [[ -n "${nobody_node_dir}" ]]; then + printf -v nobody_node_dir_for_shell '%q' "${nobody_node_dir}" + env_settings+=" PATH=${nobody_node_dir_for_shell}:\$PATH" +fi + cat >&2 </dev/null)" || nvmrc_version="" -nvmrc_version="${nvmrc_version%%[[:space:]]*}" -nvmrc_major="${nvmrc_version#v}" -nvmrc_major="${nvmrc_major%%.*}" -nobody_node_major="${nobody_node_version#v}" -nobody_node_major="${nobody_node_major%%.*}" - -if [[ "${nvmrc_major}" =~ ^[0-9]+$ && "${nobody_node_major}" != "${nvmrc_major}" ]]; then +if [[ -n "${nobody_node_dir}" ]]; then + echo "nobody's own node is ${nobody_node_version}, so the prefix runs a copy of this session's node ${session_node_version}, the version .nvmrc names and CI runs." >&2 +elif [[ "${nvmrc_major}" =~ ^[0-9]+$ && "${nobody_node_major}" != "${nvmrc_major}" ]]; then echo "nobody's node is ${nobody_node_version}, not the ${nvmrc_version} that .nvmrc names and CI runs, so a result can differ from CI's." >&2 fi diff --git a/src/__tests__/require-nonroot-tests.test.ts b/src/__tests__/require-nonroot-tests.test.ts index 98f8ff195..f1164ec89 100644 --- a/src/__tests__/require-nonroot-tests.test.ts +++ b/src/__tests__/require-nonroot-tests.test.ts @@ -4,6 +4,7 @@ import { existsSync, mkdirSync, mkdtempSync, + readdirSync, readFileSync, realpathSync, rmSync, @@ -80,9 +81,22 @@ const NOBODY_NODE_STUB = `#!/bin/sh echo "$STUB_NOBODY_NODE_VERSION" ` -/** The fixture checkout's .nvmrc, and a nobody node version that matches it. */ +/** The session's node: prints STUB_SESSION_NODE_VERSION for --version and + * hands every other call (the hook's payload parsing) to the real node at + * STUB_REAL_NODE. */ +const SESSION_NODE_STUB = `#!/bin/sh +if [ "$1" = "--version" ]; then + echo "$STUB_SESSION_NODE_VERSION" + exit 0 +fi +exec "$STUB_REAL_NODE" "$@" +` + +/** The fixture checkout's .nvmrc, and a node version that matches it. */ const NVMRC_VERSION = "24" const MATCHING_NODE_VERSION = "v24.11.0" +/** A node version whose major differs from the fixture's .nvmrc. */ +const OTHER_NODE_VERSION = "v22.22.0" const SNAPSHOTS_FOLDER = "src/vault-mcp/mcp-core/__tests__/__snapshots__" @@ -119,6 +133,11 @@ type HookRunOptions = { /** What nobody's node prints for --version. Defaults to a version that * matches the fixture's .nvmrc. */ nobodyNodeVersion?: string + /** What the session's own node prints for --version. Defaults to a + * version that matches the fixture's .nvmrc. */ + sessionNodeVersion?: string + /** The hook's NODE_EXTRA_CA_CERTS. Unset by default. */ + extraCaCerts?: string claudeProjectDir?: string /** The hook's PATH. Defaults to the stubs ahead of the runner's PATH. */ path?: string @@ -173,6 +192,7 @@ const createHookFixture = (): HookFixture => { writeExecutable(join(stubBinDir, "setpriv"), SETPRIV_STUB) writeExecutable(join(stubBinDir, "chown"), CHOWN_STUB) writeExecutable(join(nobodyBinDir, "node"), NOBODY_NODE_STUB) + writeExecutable(join(stubBinDir, "node"), SESSION_NODE_STUB) execFileSync("git", ["init", "--quiet", checkout], { env: { PATH: runnerPath(), HOME: outsideDir }, @@ -251,9 +271,12 @@ const hookEnv = (options: Omit): Record STUB_NOBODY_GID: String(gid), STUB_NOBODY_PATH: options.nobodyHasNode === false ? fixture.emptyBinDir : fixture.nobodyBinDir, STUB_NOBODY_NODE_VERSION: options.nobodyNodeVersion ?? MATCHING_NODE_VERSION, + STUB_SESSION_NODE_VERSION: options.sessionNodeVersion ?? MATCHING_NODE_VERSION, + STUB_REAL_NODE: process.execPath, STUB_CHOWN_LOG: fixture.chownLog, STUB_REAL_CHOWN: realChownPath(), ...(options.claudeProjectDir ? { CLAUDE_PROJECT_DIR: options.claudeProjectDir } : {}), + ...(options.extraCaCerts ? { NODE_EXTRA_CA_CERTS: options.extraCaCerts } : {}), } } @@ -280,20 +303,27 @@ const runCommandThroughHook = (fixture: HookFixture, command: string): HookRun = } /** The hook's reply to a stopped run, test-owned so a change to the prefix - * or the wording shows up here. nobodyHomeInPrefix is the HOME path as the - * prefix spells it: the plain path, unless the path needs quoting. */ + * or the wording shows up here. + * - nobodyHomeInPrefix is the HOME path as the prefix spells it: the plain + * path, unless the path needs quoting. + * - unsetsExtraCaCerts adds env's -u NODE_EXTRA_CA_CERTS. + * - nobodyNodeDir adds that folder first on PATH. */ const expectedRefusal = ( checkout: string, { nobodyHomeInPrefix = `${checkout}/node_modules/.nobody-home`, - }: { nobodyHomeInPrefix?: string } = {}, + unsetsExtraCaCerts = false, + nobodyNodeDir, + }: { nobodyHomeInPrefix?: string; unsetsExtraCaCerts?: boolean; nobodyNodeDir?: string } = {}, ): string => { const { uid, gid } = runnerIds() + const unsetSetting = unsetsExtraCaCerts ? "-u NODE_EXTRA_CA_CERTS " : "" + const pathSetting = nobodyNodeDir ? ` PATH=${nobodyNodeDir}:$PATH` : "" return [ "Tests do not run as root in this repo: root reads the files the permission tests make unreadable, so those tests fail here but pass in CI.", "Run the same test command as the nobody user, with this prefix in front of the test command itself (after any cd):", - ` setpriv --reuid=${uid} --regid=${gid} --clear-groups env HOME=${nobodyHomeInPrefix}`, + ` setpriv --reuid=${uid} --regid=${gid} --clear-groups env ${unsetSetting}HOME=${nobodyHomeInPrefix}${pathSetting}`, `The folders nobody needs to write in ${checkout} are ready.`, "", ].join("\n") @@ -584,13 +614,14 @@ describe("require-nonroot-tests hook", () => { expect(run).toEqual({ status: 2, stdout: "", stderr: expectedRefusal(fixture.checkout) }) }) - it("adds a line to the reply when nobody's node is another major version than .nvmrc", () => { + it("adds a line to the reply when neither nobody's node nor the session's is the .nvmrc major version", () => { const fixture = createHookFixture() const run = runHook({ fixture, stdin: payloadFor({ command: "npm test", cwd: fixture.checkout }), nobodyNodeVersion: "v22.22.0", + sessionNodeVersion: "v20.19.0", }) expect(run).toEqual({ @@ -600,6 +631,102 @@ describe("require-nonroot-tests hook", () => { }) }) + it("runs a copy of the session's node when only the session's node is the .nvmrc major version", () => { + const fixture = createHookFixture() + const nobodyNodeDir = join( + fixture.checkout, + "node_modules", + ".nobody-home", + `node-${MATCHING_NODE_VERSION}`, + ) + + const run = runHook({ + fixture, + stdin: payloadFor({ command: "npm test", cwd: fixture.checkout }), + nobodyNodeVersion: OTHER_NODE_VERSION, + }) + const copiedNode = readFileSync(join(nobodyNodeDir, "node"), "utf8") + + expect({ run, copiedNode, copyMode: permissionBits(join(nobodyNodeDir, "node")) }).toEqual({ + run: { + status: 2, + stdout: "", + stderr: `${expectedRefusal(fixture.checkout, { nobodyNodeDir })}nobody's own node is ${OTHER_NODE_VERSION}, so the prefix runs a copy of this session's node ${MATCHING_NODE_VERSION}, the version .nvmrc names and CI runs.\n`, + }, + copiedNode: SESSION_NODE_STUB, + copyMode: 0o755, + }) + }) + + it("reuses an earlier copy of the session's node instead of copying it again", () => { + const fixture = createHookFixture() + const nobodyNodeDir = join( + fixture.checkout, + "node_modules", + ".nobody-home", + `node-${MATCHING_NODE_VERSION}`, + ) + const earlierCopy = "#!/bin/sh\n# earlier copy\n" + mkdirSync(nobodyNodeDir, { recursive: true }) + writeExecutable(join(nobodyNodeDir, "node"), earlierCopy) + + const run = runHook({ + fixture, + stdin: payloadFor({ command: "npm test", cwd: fixture.checkout }), + nobodyNodeVersion: OTHER_NODE_VERSION, + }) + + expect({ + stderr: run.stderr, + copiedNode: readFileSync(join(nobodyNodeDir, "node"), "utf8"), + }).toEqual({ + stderr: `${expectedRefusal(fixture.checkout, { nobodyNodeDir })}nobody's own node is ${OTHER_NODE_VERSION}, so the prefix runs a copy of this session's node ${MATCHING_NODE_VERSION}, the version .nvmrc names and CI runs.\n`, + copiedNode: earlierCopy, + }) + }) + + it("copies no node when nobody's node is already the .nvmrc major version", () => { + const fixture = createHookFixture() + + const run = runCommandThroughHook(fixture, "npm test") + const nobodyHomeEntries = readdirSync(join(fixture.checkout, "node_modules", ".nobody-home")) + + expect({ run, nobodyHomeEntries }).toEqual({ + run: { status: 2, stdout: "", stderr: expectedRefusal(fixture.checkout) }, + nobodyHomeEntries: [], + }) + }) + + it("drops NODE_EXTRA_CA_CERTS from the prefix when nobody cannot read its file", () => { + const fixture = createHookFixture() + + const run = runHook({ + fixture, + stdin: payloadFor({ command: "npm test", cwd: fixture.checkout }), + extraCaCerts: join(fixture.outsideDir, "missing-ca-bundle.crt"), + }) + + expect(run).toEqual({ + status: 2, + stdout: "", + stderr: expectedRefusal(fixture.checkout, { unsetsExtraCaCerts: true }), + }) + }) + + it("keeps NODE_EXTRA_CA_CERTS when nobody can read its file", () => { + const fixture = createHookFixture() + const caBundle = join(fixture.outsideDir, "ca-bundle.crt") + writeFileSync(caBundle, "certificate\n") + + const run = runHook({ + fixture, + stdin: payloadFor({ command: "npm test", cwd: fixture.checkout }), + extraCaCerts: caBundle, + }) + + expect(run).toEqual({ status: 2, stdout: "", stderr: expectedRefusal(fixture.checkout) }) + }) + it("lets a test run through, preparing nothing, when nobody finds no node", () => { const fixture = createHookFixture() const stdin = payloadFor({ command: "npm test", cwd: fixture.checkout }) @@ -641,6 +768,7 @@ describe("require-nonroot-tests hook", () => { fixture, stdin: payloadFor({ command: "npm test", cwd: fixture.checkout }), nobodyNodeVersion: "v22.22.0", + sessionNodeVersion: "v20.19.0", }) expect(run).toEqual({ From 75d0397f64bec175af3d6da478d373bb09bc2d25 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 20:04:48 +0000 Subject: [PATCH 02/19] fix(hooks): find the .nvmrc node in nvm's folders when the hook's own node is another version Claude Code starts hooks with its own PATH, so in a cloud container the hook's node is the container's, not the session's nvm node. The hook now takes the newest node of the .nvmrc major version from $NVM_DIR or ~/.nvm instead. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01PkA1FHrosoEim4HpZyQGyQ --- .claude/hooks/require-nonroot-tests.sh | 44 +++++++++++++++------ src/__tests__/require-nonroot-tests.test.ts | 38 ++++++++++++++++-- 2 files changed, 66 insertions(+), 16 deletions(-) diff --git a/.claude/hooks/require-nonroot-tests.sh b/.claude/hooks/require-nonroot-tests.sh index 71c608d06..c8a410edb 100755 --- a/.claude/hooks/require-nonroot-tests.sh +++ b/.claude/hooks/require-nonroot-tests.sh @@ -312,27 +312,47 @@ nobody_writable=( ) mkdir -p "${nobody_writable[@]}" -# When nobody's node is another major version than .nvmrc names but the -# session's own node matches it, the prefix runs a copy of the session's node. -# nobody cannot run the original, because nvm installs it under root's home. +# When nobody's node is another major version than .nvmrc names, the prefix +# runs a copy of an .nvmrc-major node root has. nobody cannot run the +# original, because nvm installs it under root's home. +# - The hook's own node comes first. Claude Code starts hooks with its own +# PATH, not the session's, so in a cloud container that node is often the +# container's, and the hook then looks where nvm keeps its versions +# ($NVM_DIR or ~/.nvm), taking the newest of that major. # - The copy sits in nobody's HOME, named by version, so a later run reuses it -# and a different version gets its own folder. -# - cp -L copies the binary a symlink such as nvm's points to. +# and a different version gets its own folder. cp -L copies the binary a +# symlink points to. # - The prefix puts the copy's folder first on PATH, so the npx and tsx # processes a run starts use it too. nobody_node_dir="" if [[ "${nvmrc_major}" =~ ^[0-9]+$ && "${nobody_node_major}" != "${nvmrc_major}" ]]; then - session_node_version="$(node --version 2>/dev/null)" || session_node_version="" - session_node_major="${session_node_version#v}" - session_node_major="${session_node_major%%.*}" + ci_node_path="" + ci_node_version="$(node --version 2>/dev/null)" || ci_node_version="" + ci_node_major="${ci_node_version#v}" + ci_node_major="${ci_node_major%%.*}" + + if [[ "${ci_node_major}" == "${nvmrc_major}" ]]; then + ci_node_path="$(command -v node)" + else + for nvm_dir in "${NVM_DIR:-}" "${HOME}/.nvm"; do + [[ -n "${nvm_dir}" && -d "${nvm_dir}/versions/node" ]] || continue + # sort -V orders v24.9.0 before v24.10.0, so the last line is the newest. + newest_node="$(find "${nvm_dir}/versions/node" -mindepth 3 -maxdepth 3 -path "*/v${nvmrc_major}.*/bin/node" 2>/dev/null | sort -V | tail -n 1)" + [[ -n "${newest_node}" ]] || continue + ci_node_path="${newest_node}" + # The version is the folder name nvm gives it: /versions/node/v24.11.0/bin/node. + ci_node_version="$(basename "$(dirname "$(dirname "${newest_node}")")")" + break + done + fi - if [[ "${session_node_major}" == "${nvmrc_major}" ]]; then - nobody_node_dir="${checkout}/node_modules/.nobody-home/node-${session_node_version}" + if [[ -n "${ci_node_path}" ]]; then + nobody_node_dir="${checkout}/node_modules/.nobody-home/node-${ci_node_version}" if [[ ! -x "${nobody_node_dir}/node" ]]; then mkdir -p "${nobody_node_dir}" - cp -L "$(command -v node)" "${nobody_node_dir}/node" + cp -L "${ci_node_path}" "${nobody_node_dir}/node" fi fi fi @@ -388,7 +408,7 @@ The folders nobody needs to write in ${checkout} are ready. EOF if [[ -n "${nobody_node_dir}" ]]; then - echo "nobody's own node is ${nobody_node_version}, so the prefix runs a copy of this session's node ${session_node_version}, the version .nvmrc names and CI runs." >&2 + echo "nobody's own node is ${nobody_node_version}, so the prefix runs a copy of node ${ci_node_version}, the major version .nvmrc names and CI runs." >&2 elif [[ "${nvmrc_major}" =~ ^[0-9]+$ && "${nobody_node_major}" != "${nvmrc_major}" ]]; then echo "nobody's node is ${nobody_node_version}, not the ${nvmrc_version} that .nvmrc names and CI runs, so a result can differ from CI's." >&2 fi diff --git a/src/__tests__/require-nonroot-tests.test.ts b/src/__tests__/require-nonroot-tests.test.ts index f1164ec89..f7fc6fe46 100644 --- a/src/__tests__/require-nonroot-tests.test.ts +++ b/src/__tests__/require-nonroot-tests.test.ts @@ -631,7 +631,7 @@ describe("require-nonroot-tests hook", () => { }) }) - it("runs a copy of the session's node when only the session's node is the .nvmrc major version", () => { + it("runs a copy of the hook's own node when it is the .nvmrc major version and nobody's is not", () => { const fixture = createHookFixture() const nobodyNodeDir = join( fixture.checkout, @@ -651,14 +651,14 @@ describe("require-nonroot-tests hook", () => { run: { status: 2, stdout: "", - stderr: `${expectedRefusal(fixture.checkout, { nobodyNodeDir })}nobody's own node is ${OTHER_NODE_VERSION}, so the prefix runs a copy of this session's node ${MATCHING_NODE_VERSION}, the version .nvmrc names and CI runs.\n`, + stderr: `${expectedRefusal(fixture.checkout, { nobodyNodeDir })}nobody's own node is ${OTHER_NODE_VERSION}, so the prefix runs a copy of node ${MATCHING_NODE_VERSION}, the major version .nvmrc names and CI runs.\n`, }, copiedNode: SESSION_NODE_STUB, copyMode: 0o755, }) }) - it("reuses an earlier copy of the session's node instead of copying it again", () => { + it("reuses an earlier copy of the node instead of copying it again", () => { const fixture = createHookFixture() const nobodyNodeDir = join( fixture.checkout, @@ -680,11 +680,41 @@ describe("require-nonroot-tests hook", () => { stderr: run.stderr, copiedNode: readFileSync(join(nobodyNodeDir, "node"), "utf8"), }).toEqual({ - stderr: `${expectedRefusal(fixture.checkout, { nobodyNodeDir })}nobody's own node is ${OTHER_NODE_VERSION}, so the prefix runs a copy of this session's node ${MATCHING_NODE_VERSION}, the version .nvmrc names and CI runs.\n`, + stderr: `${expectedRefusal(fixture.checkout, { nobodyNodeDir })}nobody's own node is ${OTHER_NODE_VERSION}, so the prefix runs a copy of node ${MATCHING_NODE_VERSION}, the major version .nvmrc names and CI runs.\n`, copiedNode: earlierCopy, }) }) + it("copies nvm's newest node of the .nvmrc major version when the hook's own node is another version", () => { + const fixture = createHookFixture() + const nvmNodes = join(fixture.outsideDir, ".nvm", "versions", "node") + // v24.10.0 is newer than v24.9.0, though it sorts first as plain text. + for (const version of ["v24.9.0", "v24.10.0", "v22.22.0"]) { + mkdirSync(join(nvmNodes, version, "bin"), { recursive: true }) + writeExecutable(join(nvmNodes, version, "bin", "node"), `#!/bin/sh\n# node ${version}\n`) + } + const nobodyNodeDir = join(fixture.checkout, "node_modules", ".nobody-home", "node-v24.10.0") + + const run = runHook({ + fixture, + stdin: payloadFor({ command: "npm test", cwd: fixture.checkout }), + nobodyNodeVersion: OTHER_NODE_VERSION, + sessionNodeVersion: OTHER_NODE_VERSION, + }) + + expect({ + run, + copiedNode: readFileSync(join(nobodyNodeDir, "node"), "utf8"), + }).toEqual({ + run: { + status: 2, + stdout: "", + stderr: `${expectedRefusal(fixture.checkout, { nobodyNodeDir })}nobody's own node is ${OTHER_NODE_VERSION}, so the prefix runs a copy of node v24.10.0, the major version .nvmrc names and CI runs.\n`, + }, + copiedNode: "#!/bin/sh\n# node v24.10.0\n", + }) + }) + it("copies no node when nobody's node is already the .nvmrc major version", () => { const fixture = createHookFixture() From fda2f24f268293a4231ee410c324f7519d01d64a Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 20:06:52 +0000 Subject: [PATCH 03/19] docs(agents): say the root-test hook's prefix runs the .nvmrc Node Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01PkA1FHrosoEim4HpZyQGyQ --- AGENTS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index fbc9969fd..26508e184 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -63,7 +63,7 @@ render.yaml # Render Blueprint; stays at the repo root, the only place Render re Dockerfile # Two-target build: local (default) + remote .claude/ # Committed Claude Code hook setup only (the rest is gitignored): settings.json registers the two hooks below hooks/install-deps.sh # Runs at session start and worktree entry: loads nvm, installs dependencies when needed (npm ci, sst install), and at session start puts the checkout's Node (the .nvmrc version when nvm has it, else nvm's default alias) first on PATH for the session's later Bash commands - hooks/require-nonroot-tests.sh # Runs before each Bash command: blocks a root session's test runs, makes the folders they write writable for the nobody user, and replies to Claude with the setpriv prefix that reruns the command as nobody + hooks/require-nonroot-tests.sh # Runs before each Bash command: blocks a root session's test runs, makes the folders they write writable for the nobody user, and replies to Claude with the setpriv prefix that reruns the command as nobody, on a copy of the .nvmrc Node when nobody's own is another version obsidian-headless/ # Lockfile-pinned obsidian-headless Sync CLI for the :remote image rootfs/ # Container filesystem overlay for the :remote image: s6 init chain and services in etc/s6-overlay/, and usr/local/bin/get-sync-token, an in-container terminal sign-in to Obsidian that prints the Sync token for .env templates/memory/ # About Me/ memory file templates for new vaults From d889898b6e395f194ea3285996c80db35368dc2e Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 20:14:43 +0000 Subject: [PATCH 04/19] fix(review): probe nobody's node through env and keep the node copy whole and root-owned MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - setpriv keeps root's capabilities until it starts its command, so `setpriv … node --version` reached nvm's node under /root and reported the .nvmrc version even when nobody's runs (env, then npx's env shebang) get another. The probe now goes through /usr/bin/env, as the prefix does. - cp makes its file executable before writing it, so a copy cut short by the hook's timeout, or one a concurrent hook was still writing, passed the -x reuse check for good. The copy now lands under a temporary name and mv renames it into place. - The copy moves from nobody's HOME to node_modules/.nobody-node, a root-owned folder (755), so root no longer writes where nobody could plant a symlink, and nobody can run the copy but not replace it. Ship-Check: pr-review · claude-opus-5-5 --- .claude/hooks/require-nonroot-tests.sh | 27 ++++++++++++++++----- src/__tests__/require-nonroot-tests.test.ts | 20 +++++++++------ 2 files changed, 34 insertions(+), 13 deletions(-) diff --git a/.claude/hooks/require-nonroot-tests.sh b/.claude/hooks/require-nonroot-tests.sh index c8a410edb..693a267c9 100755 --- a/.claude/hooks/require-nonroot-tests.sh +++ b/.claude/hooks/require-nonroot-tests.sh @@ -285,7 +285,11 @@ nobody_gid="$(id -g nobody)" # root's home, where nvm installs Node, so that node can be another version # than the .nvmrc one CI runs, or missing. Without one there is no command to # offer, so the run goes through, as it does without setpriv or a nobody user. -nobody_node_version="$(setpriv --reuid="${nobody_uid}" --regid="${nobody_gid}" --clear-groups node --version 2>/dev/null)" || nobody_node_version="" +# env looks node up, as it does for the prefix: setpriv keeps root's +# capabilities until it starts its command, so its own PATH search reaches a +# node under root's home that nobody's runs cannot. /usr/bin/env is named in +# full because nobody's PATH may not hold env. +nobody_node_version="$(setpriv --reuid="${nobody_uid}" --regid="${nobody_gid}" --clear-groups /usr/bin/env node --version 2>/dev/null)" || nobody_node_version="" if [[ -z "${nobody_node_version}" ]]; then exit 0 @@ -319,9 +323,12 @@ mkdir -p "${nobody_writable[@]}" # PATH, not the session's, so in a cloud container that node is often the # container's, and the hook then looks where nvm keeps its versions # ($NVM_DIR or ~/.nvm), taking the newest of that major. -# - The copy sits in nobody's HOME, named by version, so a later run reuses it -# and a different version gets its own folder. cp -L copies the binary a -# symlink points to. +# - The copy sits in a folder named by version, so a later run reuses it and a +# different version gets its own folder. cp -L copies the binary a symlink +# points to. +# - The folder stays root's, outside the folders handed to nobody: root never +# writes where nobody could plant a symlink, and nobody can run the copy but +# not replace it. # - The prefix puts the copy's folder first on PATH, so the npx and tsx # processes a run starts use it too. nobody_node_dir="" @@ -348,11 +355,19 @@ if [[ "${nvmrc_major}" =~ ^[0-9]+$ && "${nobody_node_major}" != "${nvmrc_major}" fi if [[ -n "${ci_node_path}" ]]; then - nobody_node_dir="${checkout}/node_modules/.nobody-home/node-${ci_node_version}" + nobody_node_dir="${checkout}/node_modules/.nobody-node/node-${ci_node_version}" if [[ ! -x "${nobody_node_dir}/node" ]]; then mkdir -p "${nobody_node_dir}" - cp -L "${ci_node_path}" "${nobody_node_dir}/node" + # 755 whatever root's umask, so nobody can reach and run the copy. + chmod 755 "${nobody_node_dir%/*}" "${nobody_node_dir}" + # cp makes its file executable before writing it, so a copy cut short by + # the hook's timeout, or one a concurrent hook is still writing, would + # pass the -x check above. The copy gets a temporary name, and mv's + # rename puts only a whole binary at the name later runs reuse. + cp -L "${ci_node_path}" "${nobody_node_dir}/node.$$" + chmod 755 "${nobody_node_dir}/node.$$" + mv -f "${nobody_node_dir}/node.$$" "${nobody_node_dir}/node" fi fi fi diff --git a/src/__tests__/require-nonroot-tests.test.ts b/src/__tests__/require-nonroot-tests.test.ts index f7fc6fe46..623f84a97 100644 --- a/src/__tests__/require-nonroot-tests.test.ts +++ b/src/__tests__/require-nonroot-tests.test.ts @@ -636,7 +636,7 @@ describe("require-nonroot-tests hook", () => { const nobodyNodeDir = join( fixture.checkout, "node_modules", - ".nobody-home", + ".nobody-node", `node-${MATCHING_NODE_VERSION}`, ) @@ -647,7 +647,12 @@ describe("require-nonroot-tests hook", () => { }) const copiedNode = readFileSync(join(nobodyNodeDir, "node"), "utf8") - expect({ run, copiedNode, copyMode: permissionBits(join(nobodyNodeDir, "node")) }).toEqual({ + expect({ + run, + copiedNode, + copyMode: permissionBits(join(nobodyNodeDir, "node")), + copyFolderEntries: readdirSync(nobodyNodeDir), + }).toEqual({ run: { status: 2, stdout: "", @@ -655,6 +660,7 @@ describe("require-nonroot-tests hook", () => { }, copiedNode: SESSION_NODE_STUB, copyMode: 0o755, + copyFolderEntries: ["node"], }) }) @@ -663,7 +669,7 @@ describe("require-nonroot-tests hook", () => { const nobodyNodeDir = join( fixture.checkout, "node_modules", - ".nobody-home", + ".nobody-node", `node-${MATCHING_NODE_VERSION}`, ) const earlierCopy = "#!/bin/sh\n# earlier copy\n" @@ -693,7 +699,7 @@ describe("require-nonroot-tests hook", () => { mkdirSync(join(nvmNodes, version, "bin"), { recursive: true }) writeExecutable(join(nvmNodes, version, "bin", "node"), `#!/bin/sh\n# node ${version}\n`) } - const nobodyNodeDir = join(fixture.checkout, "node_modules", ".nobody-home", "node-v24.10.0") + const nobodyNodeDir = join(fixture.checkout, "node_modules", ".nobody-node", "node-v24.10.0") const run = runHook({ fixture, @@ -719,11 +725,11 @@ describe("require-nonroot-tests hook", () => { const fixture = createHookFixture() const run = runCommandThroughHook(fixture, "npm test") - const nobodyHomeEntries = readdirSync(join(fixture.checkout, "node_modules", ".nobody-home")) + const nodeCopyFolderMade = existsSync(join(fixture.checkout, "node_modules", ".nobody-node")) - expect({ run, nobodyHomeEntries }).toEqual({ + expect({ run, nodeCopyFolderMade }).toEqual({ run: { status: 2, stdout: "", stderr: expectedRefusal(fixture.checkout) }, - nobodyHomeEntries: [], + nodeCopyFolderMade: false, }) }) From 81a2834903cf0cfaadbcc731092a05598086655c Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 20:21:35 +0000 Subject: [PATCH 05/19] style: name the root-test hook's node copy for what it holds and say which PATH each lookup uses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Rename ci_node_* to nvmrc_node_* and hook_node_*, set the copied version where its path is set, and name the copies' parent folder. - Compute node_major_differs once and use it for both the copy and the version warning. - Build env's arguments as an array, so the reply's spacing needs no per-setting convention. - Keep the writable-folder setup in one block before the version check. - Reword the comments a stranger stopped on: the major-version match, the unset NODE_EXTRA_CA_CERTS, which PATH the probe and the prefix search, why a missing node lets the run through, the shebang lookup that makes npx and tsx use the copy, and sh's $0. Ship-Check: code-quality · claude-opus-5-5 --- .claude/hooks/require-nonroot-tests.sh | 176 ++++++++++++-------- AGENTS.md | 2 +- src/__tests__/require-nonroot-tests.test.ts | 6 +- 3 files changed, 108 insertions(+), 76 deletions(-) diff --git a/.claude/hooks/require-nonroot-tests.sh b/.claude/hooks/require-nonroot-tests.sh index 693a267c9..279f8eaaa 100755 --- a/.claude/hooks/require-nonroot-tests.sh +++ b/.claude/hooks/require-nonroot-tests.sh @@ -7,9 +7,10 @@ # exits at its first check, so it costs nothing on a developer machine. # - Before replying, it makes the folders a test run writes in the checkout # writable for nobody. -# - The prefix it offers matches CI where it can: it runs the .nvmrc Node -# when nobody's own node is another version, and it drops a -# NODE_EXTRA_CA_CERTS file nobody cannot read. +# - The prefix it offers matches CI where it can. When nobody's own node has +# another major version than .nvmrc names, the prefix runs a copy of root's +# node of that major. When nobody cannot read the file NODE_EXTRA_CA_CERTS +# names, the prefix unsets the variable. # - .claude/settings.json registers the hook on PreToolUse for Bash with a # 10-second timeout. Claude Code cancels a slower hook and runs the command # anyway, so the hook must stay fast. @@ -119,7 +120,8 @@ command_prefix="(${wrapper}|${shell_keyword}|${variable_setting})*" # followed only by spaces and then a redirection (`bash -c 'npm test' 2>`), a # ) (`$(bash -c 'npm test')`), a comment or the end. So `npm run testx` and # `npm run test:cli-pty` are not runs, and neither is the `vitest' src/` that -# `grep -E 'npm test|vitest' src/` splits into. +# `grep -E 'npm test|vitest' src/` splits into. Inside the double quotes, +# `\\)` reaches the regex as `\)`, a literal ). runner_end="([[:space:]]|\\)|[\"'][[:space:]]*([0-9]*[<>]|\\)|$)|[\"'][[:space:]]+#|$)" # A command that runs tests starts with the runner, after any prefix. A @@ -226,6 +228,8 @@ root_test_runs="" while IFS= read -r split_command; do cd_candidate="${split_command}" if [[ "${split_command}" =~ ${leading_command_prefix} ]]; then + # Keep the text after the matched prefix: ${var:offset} starts at the + # match's length. cd_candidate="${split_command:${#BASH_REMATCH[0]}}" fi if [[ "${cd_candidate}" =~ ${cd_to_absolute_path} ]]; then @@ -281,30 +285,25 @@ fi nobody_uid="$(id -u nobody)" nobody_gid="$(id -g nobody)" -# The prefix runs whichever node nobody finds on PATH. nobody cannot read -# root's home, where nvm installs Node, so that node can be another version -# than the .nvmrc one CI runs, or missing. Without one there is no command to -# offer, so the run goes through, as it does without setpriv or a nobody user. -# env looks node up, as it does for the prefix: setpriv keeps root's -# capabilities until it starts its command, so its own PATH search reaches a -# node under root's home that nobody's runs cannot. /usr/bin/env is named in -# full because nobody's PATH may not hold env. +# By default the prefix runs whichever node nobody finds on the session's +# PATH. nvm installs Node under root's home, which nobody cannot read, so +# nobody's node can be another version than the .nvmrc one CI runs, or +# missing. +# - With no node at all, nobody most likely has no npx either, because npx +# installs beside node, and the node copy made below holds only node. There +# is then no command to offer, so the run goes through, as it does without +# setpriv or a nobody user. +# - The probe searches the PATH Claude Code gives the hook, and env searches +# it as nobody, as the prefix's env does. setpriv keeps root's capabilities +# until it starts its command, so a search by setpriv itself reaches a node +# under root's home that nobody's runs cannot. /usr/bin/env is named in +# full so setpriv searches no PATH. nobody_node_version="$(setpriv --reuid="${nobody_uid}" --regid="${nobody_gid}" --clear-groups /usr/bin/env node --version 2>/dev/null)" || nobody_node_version="" if [[ -z "${nobody_node_version}" ]]; then exit 0 fi -# The major versions are compared only when .nvmrc names a number (24 or -# v24.1.0), not an alias such as lts/*. Only the file's first word counts, so -# a CRLF line end or a comment after the version does not hide it. -nvmrc_version="$(cat "${checkout}/.nvmrc" 2>/dev/null)" || nvmrc_version="" -nvmrc_version="${nvmrc_version%%[[:space:]]*}" -nvmrc_major="${nvmrc_version#v}" -nvmrc_major="${nvmrc_major%%.*}" -nobody_node_major="${nobody_node_version#v}" -nobody_node_major="${nobody_node_major%%.*}" - # A test run writes in these folders: vitest's caches under node_modules, a # fake HOME that keeps npm's cache and logs out of root's home, and coverage/ # for test:coverage's report (gitignored). @@ -314,117 +313,150 @@ nobody_writable=( "${checkout}/node_modules/.nobody-home" "${checkout}/coverage" ) + +# snapshot:update rewrites the tool-surface baseline in this folder, the only +# snapshot folder in the repo. git tracks neither owner nor write bits, so the +# changes below add nothing to a diff. +snapshots_dir="${checkout}/src/vault-mcp/mcp-core/__tests__/__snapshots__" + +if [[ -d "${snapshots_dir}" ]]; then + nobody_writable+=("${snapshots_dir}") +fi + mkdir -p "${nobody_writable[@]}" -# When nobody's node is another major version than .nvmrc names, the prefix -# runs a copy of an .nvmrc-major node root has. nobody cannot run the -# original, because nvm installs it under root's home. -# - The hook's own node comes first. Claude Code starts hooks with its own +# The folders go to nobody and are not writable by group or other users, +# because vitest runs code from its caches and checks test output against the +# snapshots. Both commands recurse, so the files a root run left inside also +# go to nobody and lose group and other write access. +chown -R "${nobody_uid}:${nobody_gid}" "${nobody_writable[@]}" +chmod -R go-w "${nobody_writable[@]}" + +# node_major_differs is true when .nvmrc names a number (24 or v24.1.0), not +# an alias such as lts/*, and nobody's node has another major version. +nvmrc_version="$(cat "${checkout}/.nvmrc" 2>/dev/null)" || nvmrc_version="" +# Cut the text at its first space, tab or line break, so a CRLF line end or a +# comment after the version does not hide it. +nvmrc_version="${nvmrc_version%%[[:space:]]*}" +nvmrc_major="${nvmrc_version#v}" +nvmrc_major="${nvmrc_major%%.*}" +nobody_node_major="${nobody_node_version#v}" +nobody_node_major="${nobody_node_major%%.*}" +node_major_differs=false + +if [[ "${nvmrc_major}" =~ ^[0-9]+$ && "${nobody_node_major}" != "${nvmrc_major}" ]]; then + node_major_differs=true +fi + +# When node_major_differs, the prefix runs a copy of a node root has +# installed whose major version .nvmrc names. nobody cannot run the original, +# because nvm installs it under root's home. +# - The hook checks its own node first. Claude Code starts hooks with its own # PATH, not the session's, so in a cloud container that node is often the -# container's, and the hook then looks where nvm keeps its versions -# ($NVM_DIR or ~/.nvm), taking the newest of that major. +# container's. The hook then looks where nvm keeps its versions ($NVM_DIR, +# then ~/.nvm, often the same folder) and takes the newest of that major in +# the first folder that has one. # - The copy sits in a folder named by version, so a later run reuses it and a # different version gets its own folder. cp -L copies the binary a symlink # points to. # - The folder stays root's, outside the folders handed to nobody: root never # writes where nobody could plant a symlink, and nobody can run the copy but # not replace it. -# - The prefix puts the copy's folder first on PATH, so the npx and tsx -# processes a run starts use it too. +# - The prefix puts the copy's folder first on PATH. nobody's runs then find +# the copy, and so do the npx and tsx scripts a run starts, whose +# `#!/usr/bin/env node` lines look node up on PATH. npx and npm themselves +# still come from the session's PATH. nobody_node_dir="" -if [[ "${nvmrc_major}" =~ ^[0-9]+$ && "${nobody_node_major}" != "${nvmrc_major}" ]]; then - ci_node_path="" - ci_node_version="$(node --version 2>/dev/null)" || ci_node_version="" - ci_node_major="${ci_node_version#v}" - ci_node_major="${ci_node_major%%.*}" +if [[ "${node_major_differs}" == true ]]; then + nvmrc_node_path="" + nvmrc_node_version="" + hook_node_version="$(node --version 2>/dev/null)" || hook_node_version="" + hook_node_major="${hook_node_version#v}" + hook_node_major="${hook_node_major%%.*}" - if [[ "${ci_node_major}" == "${nvmrc_major}" ]]; then - ci_node_path="$(command -v node)" + if [[ "${hook_node_major}" == "${nvmrc_major}" ]]; then + nvmrc_node_path="$(command -v node)" + nvmrc_node_version="${hook_node_version}" else for nvm_dir in "${NVM_DIR:-}" "${HOME}/.nvm"; do [[ -n "${nvm_dir}" && -d "${nvm_dir}/versions/node" ]] || continue + # nvm keeps each node three folders below versions/node, in a folder + # named by its version: /versions/node/v24.11.0/bin/node. # sort -V orders v24.9.0 before v24.10.0, so the last line is the newest. newest_node="$(find "${nvm_dir}/versions/node" -mindepth 3 -maxdepth 3 -path "*/v${nvmrc_major}.*/bin/node" 2>/dev/null | sort -V | tail -n 1)" [[ -n "${newest_node}" ]] || continue - ci_node_path="${newest_node}" - # The version is the folder name nvm gives it: /versions/node/v24.11.0/bin/node. - ci_node_version="$(basename "$(dirname "$(dirname "${newest_node}")")")" + nvmrc_node_path="${newest_node}" + nvmrc_node_version="$(basename "$(dirname "$(dirname "${newest_node}")")")" break done fi - if [[ -n "${ci_node_path}" ]]; then - nobody_node_dir="${checkout}/node_modules/.nobody-node/node-${ci_node_version}" + if [[ -n "${nvmrc_node_path}" ]]; then + node_copies_dir="${checkout}/node_modules/.nobody-node" + nobody_node_dir="${node_copies_dir}/node-${nvmrc_node_version}" + # A node already at this name is reused. Only the mv below puts one + # there, so it is a whole copy. if [[ ! -x "${nobody_node_dir}/node" ]]; then mkdir -p "${nobody_node_dir}" # 755 whatever root's umask, so nobody can reach and run the copy. - chmod 755 "${nobody_node_dir%/*}" "${nobody_node_dir}" + chmod 755 "${node_copies_dir}" "${nobody_node_dir}" # cp makes its file executable before writing it, so a copy cut short by # the hook's timeout, or one a concurrent hook is still writing, would # pass the -x check above. The copy gets a temporary name, and mv's # rename puts only a whole binary at the name later runs reuse. - cp -L "${ci_node_path}" "${nobody_node_dir}/node.$$" + cp -L "${nvmrc_node_path}" "${nobody_node_dir}/node.$$" chmod 755 "${nobody_node_dir}/node.$$" mv -f "${nobody_node_dir}/node.$$" "${nobody_node_dir}/node" fi fi fi -# snapshot:update rewrites the tool-surface baseline in this folder, the only -# snapshot folder in the repo. git tracks neither owner nor write bits, so the -# changes below add nothing to a diff. -snapshots_dir="${checkout}/src/vault-mcp/mcp-core/__tests__/__snapshots__" - -if [[ -d "${snapshots_dir}" ]]; then - nobody_writable+=("${snapshots_dir}") -fi - -# The folders go to nobody and are not writable by group or other users, -# because vitest runs code from its caches and checks test output against the -# snapshots. Both commands recurse, so the files a root run left inside also -# go to nobody and lose group and other write access. -chown -R "${nobody_uid}:${nobody_gid}" "${nobody_writable[@]}" -chmod -R go-w "${nobody_writable[@]}" - # %q quotes the HOME path for the shell, so the prefix still works when the # checkout's path holds a space. A path of plain characters prints unchanged. printf -v nobody_home_for_shell '%q' "${checkout}/node_modules/.nobody-home" -# env's settings, each followed by a space. $PATH stays literal, so the shell -# running the prefix expands it. -env_settings="" +# env's arguments, joined with spaces in the reply. The -u option goes first, +# because env reads options only before its first NAME=value setting. +env_arguments=() # Claude Code's cloud containers point NODE_EXTRA_CA_CERTS at a file under # root's home. Every node nobody starts then prints a warning that it cannot # read the file, which breaks tests that check a child process's exact stderr. -# The tests make no network calls, so the prefix drops the setting. /bin/sh is -# named in full because nobody's PATH may not hold sh, and its $1 is -# single-quoted on purpose: the inner shell expands it, not this one. +# The suites this prefix runs make no network calls, so the prefix unsets the +# variable. +# - /bin/sh is named in full so setpriv searches no PATH. +# - The script's $1 is single-quoted on purpose: the inner shell expands it, +# not this one. The `sh` after the script is the inner shell's $0, so the +# file's path lands in $1. # shellcheck disable=SC2016 if [[ -n "${NODE_EXTRA_CA_CERTS:-}" ]] && ! setpriv --reuid="${nobody_uid}" --regid="${nobody_gid}" --clear-groups /bin/sh -c 'test -r "$1"' sh "${NODE_EXTRA_CA_CERTS}"; then - env_settings+="-u NODE_EXTRA_CA_CERTS " + env_arguments+=(-u NODE_EXTRA_CA_CERTS) fi -env_settings+="HOME=${nobody_home_for_shell}" +env_arguments+=("HOME=${nobody_home_for_shell}") +# $PATH stays literal, so the shell running the prefix expands it to the +# session's PATH, which env searches after the copy's folder. A folder there +# that nobody cannot read is skipped. if [[ -n "${nobody_node_dir}" ]]; then printf -v nobody_node_dir_for_shell '%q' "${nobody_node_dir}" - env_settings+=" PATH=${nobody_node_dir_for_shell}:\$PATH" + env_arguments+=("PATH=${nobody_node_dir_for_shell}:\$PATH") fi cat >&2 <&2 -elif [[ "${nvmrc_major}" =~ ^[0-9]+$ && "${nobody_node_major}" != "${nvmrc_major}" ]]; then + echo "nobody's own node is ${nobody_node_version}, so the prefix runs a copy of node ${nvmrc_node_version}, whose major version .nvmrc names and CI runs." >&2 +elif [[ "${node_major_differs}" == true ]]; then + # The majors differ, but root has no node of the .nvmrc major to copy. echo "nobody's node is ${nobody_node_version}, not the ${nvmrc_version} that .nvmrc names and CI runs, so a result can differ from CI's." >&2 fi diff --git a/AGENTS.md b/AGENTS.md index 26508e184..1287677f4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -63,7 +63,7 @@ render.yaml # Render Blueprint; stays at the repo root, the only place Render re Dockerfile # Two-target build: local (default) + remote .claude/ # Committed Claude Code hook setup only (the rest is gitignored): settings.json registers the two hooks below hooks/install-deps.sh # Runs at session start and worktree entry: loads nvm, installs dependencies when needed (npm ci, sst install), and at session start puts the checkout's Node (the .nvmrc version when nvm has it, else nvm's default alias) first on PATH for the session's later Bash commands - hooks/require-nonroot-tests.sh # Runs before each Bash command: blocks a root session's test runs, makes the folders they write writable for the nobody user, and replies to Claude with the setpriv prefix that reruns the command as nobody, on a copy of the .nvmrc Node when nobody's own is another version + hooks/require-nonroot-tests.sh # Runs before each Bash command: blocks a root session's test runs, makes the folders they write writable for the nobody user, and replies to Claude with the setpriv prefix that reruns the command as nobody. When nobody's own Node has another major version than .nvmrc names, the prefix runs a copy of root's Node of that major obsidian-headless/ # Lockfile-pinned obsidian-headless Sync CLI for the :remote image rootfs/ # Container filesystem overlay for the :remote image: s6 init chain and services in etc/s6-overlay/, and usr/local/bin/get-sync-token, an in-container terminal sign-in to Obsidian that prints the Sync token for .env templates/memory/ # About Me/ memory file templates for new vaults diff --git a/src/__tests__/require-nonroot-tests.test.ts b/src/__tests__/require-nonroot-tests.test.ts index 623f84a97..55963ff40 100644 --- a/src/__tests__/require-nonroot-tests.test.ts +++ b/src/__tests__/require-nonroot-tests.test.ts @@ -656,7 +656,7 @@ describe("require-nonroot-tests hook", () => { run: { status: 2, stdout: "", - stderr: `${expectedRefusal(fixture.checkout, { nobodyNodeDir })}nobody's own node is ${OTHER_NODE_VERSION}, so the prefix runs a copy of node ${MATCHING_NODE_VERSION}, the major version .nvmrc names and CI runs.\n`, + stderr: `${expectedRefusal(fixture.checkout, { nobodyNodeDir })}nobody's own node is ${OTHER_NODE_VERSION}, so the prefix runs a copy of node ${MATCHING_NODE_VERSION}, whose major version .nvmrc names and CI runs.\n`, }, copiedNode: SESSION_NODE_STUB, copyMode: 0o755, @@ -686,7 +686,7 @@ describe("require-nonroot-tests hook", () => { stderr: run.stderr, copiedNode: readFileSync(join(nobodyNodeDir, "node"), "utf8"), }).toEqual({ - stderr: `${expectedRefusal(fixture.checkout, { nobodyNodeDir })}nobody's own node is ${OTHER_NODE_VERSION}, so the prefix runs a copy of node ${MATCHING_NODE_VERSION}, the major version .nvmrc names and CI runs.\n`, + stderr: `${expectedRefusal(fixture.checkout, { nobodyNodeDir })}nobody's own node is ${OTHER_NODE_VERSION}, so the prefix runs a copy of node ${MATCHING_NODE_VERSION}, whose major version .nvmrc names and CI runs.\n`, copiedNode: earlierCopy, }) }) @@ -715,7 +715,7 @@ describe("require-nonroot-tests hook", () => { run: { status: 2, stdout: "", - stderr: `${expectedRefusal(fixture.checkout, { nobodyNodeDir })}nobody's own node is ${OTHER_NODE_VERSION}, so the prefix runs a copy of node v24.10.0, the major version .nvmrc names and CI runs.\n`, + stderr: `${expectedRefusal(fixture.checkout, { nobodyNodeDir })}nobody's own node is ${OTHER_NODE_VERSION}, so the prefix runs a copy of node v24.10.0, whose major version .nvmrc names and CI runs.\n`, }, copiedNode: "#!/bin/sh\n# node v24.10.0\n", }) From 3acb0d6f7796bc00e28e5994edfd9eac7c7aeba5 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 20:26:27 +0000 Subject: [PATCH 06/19] test(hooks): model setpriv's own PATH search and cover the node copy's edges MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - The setpriv stub now looks a bare command up on the hook's PATH, as real setpriv does with root's capabilities, and leaves a command named by path to search nobody's PATH. A probe that drops /usr/bin/env now fails the tests. - New cases: a copy cut short is copied again, $NVM_DIR wins over ~/.nvm and falls through to it, a full .nvmrc version compares by major, the copy and its folders stay 0755 under umask 077, the copy's folder is quoted in the prefix, and the copy is never handed to nobody. - The reuse test asserts the whole run. Ship-Check: test-audit · claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 --- src/__tests__/require-nonroot-tests.test.ts | 263 ++++++++++++++++++-- 1 file changed, 238 insertions(+), 25 deletions(-) diff --git a/src/__tests__/require-nonroot-tests.test.ts b/src/__tests__/require-nonroot-tests.test.ts index 55963ff40..cbb09423a 100644 --- a/src/__tests__/require-nonroot-tests.test.ts +++ b/src/__tests__/require-nonroot-tests.test.ts @@ -54,7 +54,12 @@ esac /** Stub `setpriv`: drops its own options and runs the rest of the command * with STUB_NOBODY_PATH as PATH, standing in for the PATH entries nobody - * can read. */ + * can read. + * - A bare command name is looked up on the PATH the stub was given, before + * the switch. Real setpriv keeps root's capabilities until it starts its + * command, so its own search reaches folders nobody cannot read. + * - A command named by path runs as named, and searches nobody's PATH + * itself (`/usr/bin/env node`). */ const SETPRIV_STUB = `#!/bin/sh while [ "$#" -gt 0 ]; do case "$1" in @@ -62,9 +67,14 @@ while [ "$#" -gt 0 ]; do *) break ;; esac done +case "$1" in + */*) command_path="$1" ;; + *) command_path="$(command -v "$1")" || exit 127 ;; +esac +shift PATH="$STUB_NOBODY_PATH" export PATH -exec "$@" +exec "$command_path" "$@" ` /** Stub `chown`: appends its arguments to STUB_CHOWN_LOG, one call per line, @@ -92,6 +102,16 @@ fi exec "$STUB_REAL_NODE" "$@" ` +/** A `cp` cut short, as by the hook's timeout: like the real cp, it makes + * its last argument executable before writing it, then stops partway. */ +const CUT_SHORT_CP_STUB = `#!/bin/sh +for destination do :; done +: > "$destination" +chmod 755 "$destination" +printf '#!/bin/sh\\n# partial' > "$destination" +exit 1 +` + /** The fixture checkout's .nvmrc, and a node version that matches it. */ const NVMRC_VERSION = "24" const MATCHING_NODE_VERSION = "v24.11.0" @@ -138,6 +158,11 @@ type HookRunOptions = { sessionNodeVersion?: string /** The hook's NODE_EXTRA_CA_CERTS. Unset by default. */ extraCaCerts?: string + /** The hook's NVM_DIR. Unset by default, so the hook looks only in ~/.nvm. */ + nvmDir?: string + /** The umask the hook starts with, as `umask` takes it. Defaults to the + * runner's own. */ + umask?: string claudeProjectDir?: string /** The hook's PATH. Defaults to the stubs ahead of the runner's PATH. */ path?: string @@ -277,11 +302,16 @@ const hookEnv = (options: Omit): Record STUB_REAL_CHOWN: realChownPath(), ...(options.claudeProjectDir ? { CLAUDE_PROJECT_DIR: options.claudeProjectDir } : {}), ...(options.extraCaCerts ? { NODE_EXTRA_CA_CERTS: options.extraCaCerts } : {}), + ...(options.nvmDir ? { NVM_DIR: options.nvmDir } : {}), } } const runHook = (options: HookRunOptions): HookRun => { - const result = spawnSync("bash", [HOOK_PATH], { + // The script's "$0" is the hook's path, which bash -c takes after the script. + const hookArguments = options.umask + ? ["-c", `umask ${options.umask} && exec bash "$0"`, HOOK_PATH] + : [HOOK_PATH] + const result = spawnSync("bash", hookArguments, { // Outside any checkout, so the hook's "." fallback can never reach the // repo running these tests. cwd: options.fixture.outsideDir, @@ -329,6 +359,32 @@ const expectedRefusal = ( ].join("\n") } +/** The reply's line when the prefix runs a copy of node. */ +const expectedNodeCopyLine = ({ + nobodyNodeVersion, + copiedNodeVersion, +}: { + nobodyNodeVersion: string + copiedNodeVersion: string +}): string => { + return `nobody's own node is ${nobodyNodeVersion}, so the prefix runs a copy of node ${copiedNodeVersion}, whose major version .nvmrc names and CI runs.\n` +} + +/** The folder the hook copies a node of this version into. */ +const nodeCopyFolder = (checkout: string, nodeVersion: string): string => { + return join(checkout, "node_modules", ".nobody-node", `node-${nodeVersion}`) +} + +/** Lays out nvm's folders under nvmDir, one stub node per version, each + * printing its version so a copy shows which one it came from. */ +const writeNvmNodes = (nvmDir: string, nodeVersions: readonly string[]): void => { + for (const nodeVersion of nodeVersions) { + const binDir = join(nvmDir, "versions", "node", nodeVersion, "bin") + mkdirSync(binDir, { recursive: true }) + writeExecutable(join(binDir, "node"), `#!/bin/sh\n# node ${nodeVersion}\n`) + } +} + const scratchFolders = (checkout: string): string[] => [ join(checkout, "node_modules", ".vite-temp"), join(checkout, "node_modules", ".vitest"), @@ -633,12 +689,8 @@ describe("require-nonroot-tests hook", () => { it("runs a copy of the hook's own node when it is the .nvmrc major version and nobody's is not", () => { const fixture = createHookFixture() - const nobodyNodeDir = join( - fixture.checkout, - "node_modules", - ".nobody-node", - `node-${MATCHING_NODE_VERSION}`, - ) + const { uid, gid } = runnerIds() + const nobodyNodeDir = nodeCopyFolder(fixture.checkout, MATCHING_NODE_VERSION) const run = runHook({ fixture, @@ -647,31 +699,48 @@ describe("require-nonroot-tests hook", () => { }) const copiedNode = readFileSync(join(nobodyNodeDir, "node"), "utf8") + // The chown calls leave the copy out: it stays root's, so nobody can + // run it but not replace it. expect({ run, copiedNode, copyMode: permissionBits(join(nobodyNodeDir, "node")), copyFolderEntries: readdirSync(nobodyNodeDir), + chownCalls: recordedChownCalls(fixture), }).toEqual({ run: { status: 2, stdout: "", - stderr: `${expectedRefusal(fixture.checkout, { nobodyNodeDir })}nobody's own node is ${OTHER_NODE_VERSION}, so the prefix runs a copy of node ${MATCHING_NODE_VERSION}, whose major version .nvmrc names and CI runs.\n`, + stderr: `${expectedRefusal(fixture.checkout, { nobodyNodeDir })}${expectedNodeCopyLine({ nobodyNodeVersion: OTHER_NODE_VERSION, copiedNodeVersion: MATCHING_NODE_VERSION })}`, }, copiedNode: SESSION_NODE_STUB, copyMode: 0o755, copyFolderEntries: ["node"], + chownCalls: [`-R ${uid}:${gid} ${scratchFolders(fixture.checkout).join(" ")}`], }) }) + it("makes the node copy and its folders readable and runnable by others under a restrictive umask", () => { + const fixture = createHookFixture() + const nodeCopiesDir = join(fixture.checkout, "node_modules", ".nobody-node") + const nobodyNodeDir = nodeCopyFolder(fixture.checkout, MATCHING_NODE_VERSION) + + const run = runHook({ + fixture, + stdin: payloadFor({ command: "npm test", cwd: fixture.checkout }), + nobodyNodeVersion: OTHER_NODE_VERSION, + umask: "077", + }) + + expect({ + status: run.status, + modes: [nodeCopiesDir, nobodyNodeDir, join(nobodyNodeDir, "node")].map(permissionBits), + }).toEqual({ status: 2, modes: [0o755, 0o755, 0o755] }) + }) + it("reuses an earlier copy of the node instead of copying it again", () => { const fixture = createHookFixture() - const nobodyNodeDir = join( - fixture.checkout, - "node_modules", - ".nobody-node", - `node-${MATCHING_NODE_VERSION}`, - ) + const nobodyNodeDir = nodeCopyFolder(fixture.checkout, MATCHING_NODE_VERSION) const earlierCopy = "#!/bin/sh\n# earlier copy\n" mkdirSync(nobodyNodeDir, { recursive: true }) writeExecutable(join(nobodyNodeDir, "node"), earlierCopy) @@ -683,29 +752,90 @@ describe("require-nonroot-tests hook", () => { }) expect({ - stderr: run.stderr, + run, copiedNode: readFileSync(join(nobodyNodeDir, "node"), "utf8"), }).toEqual({ - stderr: `${expectedRefusal(fixture.checkout, { nobodyNodeDir })}nobody's own node is ${OTHER_NODE_VERSION}, so the prefix runs a copy of node ${MATCHING_NODE_VERSION}, whose major version .nvmrc names and CI runs.\n`, + run: { + status: 2, + stdout: "", + stderr: `${expectedRefusal(fixture.checkout, { nobodyNodeDir })}${expectedNodeCopyLine({ nobodyNodeVersion: OTHER_NODE_VERSION, copiedNodeVersion: MATCHING_NODE_VERSION })}`, + }, copiedNode: earlierCopy, }) }) + it("copies the node again when an earlier copy was cut short", () => { + const fixture = createHookFixture() + const nobodyNodeDir = nodeCopyFolder(fixture.checkout, MATCHING_NODE_VERSION) + const cutShortBinDir = join(fixture.outsideDir, "cut-short-bin") + mkdirSync(cutShortBinDir) + writeExecutable(join(cutShortBinDir, "cp"), CUT_SHORT_CP_STUB) + const stdin = payloadFor({ command: "npm test", cwd: fixture.checkout }) + + const cutShortRun = runHook({ + fixture, + stdin, + nobodyNodeVersion: OTHER_NODE_VERSION, + path: `${cutShortBinDir}:${fixture.stubBinDir}:${runnerPath()}`, + }) + const run = runHook({ fixture, stdin, nobodyNodeVersion: OTHER_NODE_VERSION }) + + // The cut-short run fails at its cp, so the second run meets the + // partial file it left. + expect({ + cutShortStatus: cutShortRun.status, + run, + copiedNode: readFileSync(join(nobodyNodeDir, "node"), "utf8"), + }).toEqual({ + cutShortStatus: 1, + run: { + status: 2, + stdout: "", + stderr: `${expectedRefusal(fixture.checkout, { nobodyNodeDir })}${expectedNodeCopyLine({ nobodyNodeVersion: OTHER_NODE_VERSION, copiedNodeVersion: MATCHING_NODE_VERSION })}`, + }, + copiedNode: SESSION_NODE_STUB, + }) + }) + it("copies nvm's newest node of the .nvmrc major version when the hook's own node is another version", () => { const fixture = createHookFixture() - const nvmNodes = join(fixture.outsideDir, ".nvm", "versions", "node") // v24.10.0 is newer than v24.9.0, though it sorts first as plain text. - for (const version of ["v24.9.0", "v24.10.0", "v22.22.0"]) { - mkdirSync(join(nvmNodes, version, "bin"), { recursive: true }) - writeExecutable(join(nvmNodes, version, "bin", "node"), `#!/bin/sh\n# node ${version}\n`) - } - const nobodyNodeDir = join(fixture.checkout, "node_modules", ".nobody-node", "node-v24.10.0") + writeNvmNodes(join(fixture.outsideDir, ".nvm"), ["v24.9.0", "v24.10.0", "v22.22.0"]) + const nobodyNodeDir = nodeCopyFolder(fixture.checkout, "v24.10.0") + + const run = runHook({ + fixture, + stdin: payloadFor({ command: "npm test", cwd: fixture.checkout }), + nobodyNodeVersion: OTHER_NODE_VERSION, + sessionNodeVersion: OTHER_NODE_VERSION, + }) + + expect({ + run, + copiedNode: readFileSync(join(nobodyNodeDir, "node"), "utf8"), + }).toEqual({ + run: { + status: 2, + stdout: "", + stderr: `${expectedRefusal(fixture.checkout, { nobodyNodeDir })}${expectedNodeCopyLine({ nobodyNodeVersion: OTHER_NODE_VERSION, copiedNodeVersion: "v24.10.0" })}`, + }, + copiedNode: "#!/bin/sh\n# node v24.10.0\n", + }) + }) + + it("copies the node in $NVM_DIR before a newer one in ~/.nvm", () => { + const fixture = createHookFixture() + const nvmDir = join(fixture.outsideDir, "nvm-dir") + writeNvmNodes(nvmDir, ["v24.1.0"]) + writeNvmNodes(join(fixture.outsideDir, ".nvm"), ["v24.10.0"]) + const nobodyNodeDir = nodeCopyFolder(fixture.checkout, "v24.1.0") const run = runHook({ fixture, stdin: payloadFor({ command: "npm test", cwd: fixture.checkout }), nobodyNodeVersion: OTHER_NODE_VERSION, sessionNodeVersion: OTHER_NODE_VERSION, + nvmDir, }) expect({ @@ -715,12 +845,93 @@ describe("require-nonroot-tests hook", () => { run: { status: 2, stdout: "", - stderr: `${expectedRefusal(fixture.checkout, { nobodyNodeDir })}nobody's own node is ${OTHER_NODE_VERSION}, so the prefix runs a copy of node v24.10.0, whose major version .nvmrc names and CI runs.\n`, + stderr: `${expectedRefusal(fixture.checkout, { nobodyNodeDir })}${expectedNodeCopyLine({ nobodyNodeVersion: OTHER_NODE_VERSION, copiedNodeVersion: "v24.1.0" })}`, + }, + copiedNode: "#!/bin/sh\n# node v24.1.0\n", + }) + }) + + it("copies the node in ~/.nvm when $NVM_DIR has none of the .nvmrc major version", () => { + const fixture = createHookFixture() + const nvmDir = join(fixture.outsideDir, "nvm-dir") + writeNvmNodes(nvmDir, ["v22.22.0"]) + writeNvmNodes(join(fixture.outsideDir, ".nvm"), ["v24.10.0"]) + const nobodyNodeDir = nodeCopyFolder(fixture.checkout, "v24.10.0") + + const run = runHook({ + fixture, + stdin: payloadFor({ command: "npm test", cwd: fixture.checkout }), + nobodyNodeVersion: OTHER_NODE_VERSION, + sessionNodeVersion: OTHER_NODE_VERSION, + nvmDir, + }) + + expect({ + run, + copiedNode: readFileSync(join(nobodyNodeDir, "node"), "utf8"), + }).toEqual({ + run: { + status: 2, + stdout: "", + stderr: `${expectedRefusal(fixture.checkout, { nobodyNodeDir })}${expectedNodeCopyLine({ nobodyNodeVersion: OTHER_NODE_VERSION, copiedNodeVersion: "v24.10.0" })}`, }, copiedNode: "#!/bin/sh\n# node v24.10.0\n", }) }) + it("copies a node of the major version a full .nvmrc version names", () => { + const fixture = createHookFixture() + writeFileSync(join(fixture.checkout, ".nvmrc"), "v24.1.0\n") + const nobodyNodeDir = nodeCopyFolder(fixture.checkout, MATCHING_NODE_VERSION) + + const run = runHook({ + fixture, + stdin: payloadFor({ command: "npm test", cwd: fixture.checkout }), + nobodyNodeVersion: OTHER_NODE_VERSION, + }) + + expect(run).toEqual({ + status: 2, + stdout: "", + stderr: `${expectedRefusal(fixture.checkout, { nobodyNodeDir })}${expectedNodeCopyLine({ nobodyNodeVersion: OTHER_NODE_VERSION, copiedNodeVersion: MATCHING_NODE_VERSION })}`, + }) + }) + + it("copies no node when nobody's node has the major version a full .nvmrc version names", () => { + const fixture = createHookFixture() + writeFileSync(join(fixture.checkout, ".nvmrc"), "v24.1.0\n") + + const run = runCommandThroughHook(fixture, "npm test") + const nodeCopyFolderMade = existsSync(join(fixture.checkout, "node_modules", ".nobody-node")) + + expect({ run, nodeCopyFolderMade }).toEqual({ + run: { status: 2, stdout: "", stderr: expectedRefusal(fixture.checkout) }, + nodeCopyFolderMade: false, + }) + }) + + it("quotes the node copy's folder in the prefix when the checkout's path holds a space", () => { + const fixture = createHookFixture() + const spacedCheckout = createOtherCheckout(fixture, "spaced checkout") + writeFileSync(join(spacedCheckout, ".nvmrc"), `${NVMRC_VERSION}\n`) + const escapedCheckout = spacedCheckout.replaceAll(" ", "\\ ") + + const run = runHook({ + fixture, + stdin: payloadFor({ command: "npm test", cwd: spacedCheckout }), + nobodyNodeVersion: OTHER_NODE_VERSION, + }) + + expect(run).toEqual({ + status: 2, + stdout: "", + stderr: `${expectedRefusal(spacedCheckout, { + nobodyHomeInPrefix: `${escapedCheckout}/node_modules/.nobody-home`, + nobodyNodeDir: nodeCopyFolder(escapedCheckout, MATCHING_NODE_VERSION), + })}${expectedNodeCopyLine({ nobodyNodeVersion: OTHER_NODE_VERSION, copiedNodeVersion: MATCHING_NODE_VERSION })}`, + }) + }) + it("copies no node when nobody's node is already the .nvmrc major version", () => { const fixture = createHookFixture() @@ -767,6 +978,8 @@ describe("require-nonroot-tests hook", () => { const fixture = createHookFixture() const stdin = payloadFor({ command: "npm test", cwd: fixture.checkout }) + // The hook's own PATH still has a node, which a search by setpriv + // itself, with root's capabilities, would find. const run = runHook({ fixture, stdin, nobodyHasNode: false }) const foldersMade = scratchFolders(fixture.checkout).filter((folder) => existsSync(folder)) const controlRun = runHook({ fixture, stdin }) From eeb2410a8b40ca3586adc2ebe17e69f725b1ab04 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 20:49:41 +0000 Subject: [PATCH 07/19] fix: quote PATH in the root-test hook's prefix and copy the binary node runs from MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - The prefix printed `PATH=:$PATH` with $PATH unquoted. bash splits it when a folder on the session's PATH holds a space (WSL appends Windows folders such as "Program Files"), and env then takes the text after the space as its command. The prefix now prints `:"$PATH"`. - When the hook's own node matched .nvmrc, the hook copied `command -v node`, which can be a version manager's shim (asdf, mise, volta). The copy then fails as nobody, and the -x reuse check keeps the broken copy. The hook now copies `node -p process.execPath`, the binary node runs from. - A failed cp (a full disk) left its partial temporary file behind, holding the space it took. The hook now removes it before ending as set -e would. - find -L enters a symlinked versions/node or version folder under nvm's folders. - AGENTS.md: the prefix runs a copy of root's Node of the .nvmrc major only if root has one. Ship-Check: bug-check · claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 --- .claude/hooks/require-nonroot-tests.sh | 20 ++++-- AGENTS.md | 2 +- src/__tests__/require-nonroot-tests.test.ts | 72 +++++++++++++++++++-- 3 files changed, 84 insertions(+), 10 deletions(-) diff --git a/.claude/hooks/require-nonroot-tests.sh b/.claude/hooks/require-nonroot-tests.sh index 279f8eaaa..2ef4c1563 100755 --- a/.claude/hooks/require-nonroot-tests.sh +++ b/.claude/hooks/require-nonroot-tests.sh @@ -376,7 +376,10 @@ if [[ "${node_major_differs}" == true ]]; then hook_node_major="${hook_node_major%%.*}" if [[ "${hook_node_major}" == "${nvmrc_major}" ]]; then - nvmrc_node_path="$(command -v node)" + # node names the binary it runs from. The node on PATH can be a version + # manager's shim (asdf, mise, volta) that starts it, and a copy of the + # shim fails as nobody, who cannot read the version manager's files. + nvmrc_node_path="$(node -p process.execPath 2>/dev/null)" || nvmrc_node_path="" nvmrc_node_version="${hook_node_version}" else for nvm_dir in "${NVM_DIR:-}" "${HOME}/.nvm"; do @@ -384,7 +387,8 @@ if [[ "${node_major_differs}" == true ]]; then # nvm keeps each node three folders below versions/node, in a folder # named by its version: /versions/node/v24.11.0/bin/node. # sort -V orders v24.9.0 before v24.10.0, so the last line is the newest. - newest_node="$(find "${nvm_dir}/versions/node" -mindepth 3 -maxdepth 3 -path "*/v${nvmrc_major}.*/bin/node" 2>/dev/null | sort -V | tail -n 1)" + # -L enters a versions/node or version folder that is a symlink. + newest_node="$(find -L "${nvm_dir}/versions/node" -mindepth 3 -maxdepth 3 -path "*/v${nvmrc_major}.*/bin/node" 2>/dev/null | sort -V | tail -n 1)" [[ -n "${newest_node}" ]] || continue nvmrc_node_path="${newest_node}" nvmrc_node_version="$(basename "$(dirname "$(dirname "${newest_node}")")")" @@ -406,7 +410,12 @@ if [[ "${node_major_differs}" == true ]]; then # the hook's timeout, or one a concurrent hook is still writing, would # pass the -x check above. The copy gets a temporary name, and mv's # rename puts only a whole binary at the name later runs reuse. - cp -L "${nvmrc_node_path}" "${nobody_node_dir}/node.$$" + # A failed cp (a full disk) ends the hook as set -e would, after + # removing its partial file, which would otherwise hold the space. + cp -L "${nvmrc_node_path}" "${nobody_node_dir}/node.$$" || { + rm -f "${nobody_node_dir}/node.$$" + exit 1 + } chmod 755 "${nobody_node_dir}/node.$$" mv -f "${nobody_node_dir}/node.$$" "${nobody_node_dir}/node" fi @@ -441,9 +450,12 @@ env_arguments+=("HOME=${nobody_home_for_shell}") # $PATH stays literal, so the shell running the prefix expands it to the # session's PATH, which env searches after the copy's folder. A folder there # that nobody cannot read is skipped. +# - The double quotes keep that PATH one argument when a folder in it holds a +# space, as Windows folders on WSL's PATH do. Unquoted, the shell would +# split it, and env would take the text after the space as its command. if [[ -n "${nobody_node_dir}" ]]; then printf -v nobody_node_dir_for_shell '%q' "${nobody_node_dir}" - env_arguments+=("PATH=${nobody_node_dir_for_shell}:\$PATH") + env_arguments+=("PATH=${nobody_node_dir_for_shell}:\"\$PATH\"") fi cat >&2 <): Record ...(options.claudeProjectDir ? { CLAUDE_PROJECT_DIR: options.claudeProjectDir } : {}), ...(options.extraCaCerts ? { NODE_EXTRA_CA_CERTS: options.extraCaCerts } : {}), ...(options.nvmDir ? { NVM_DIR: options.nvmDir } : {}), + ...(options.sessionNodeBinary ? { STUB_SESSION_NODE_BINARY: options.sessionNodeBinary } : {}), } } @@ -348,7 +358,7 @@ const expectedRefusal = ( ): string => { const { uid, gid } = runnerIds() const unsetSetting = unsetsExtraCaCerts ? "-u NODE_EXTRA_CA_CERTS " : "" - const pathSetting = nobodyNodeDir ? ` PATH=${nobodyNodeDir}:$PATH` : "" + const pathSetting = nobodyNodeDir ? ` PATH=${nobodyNodeDir}:"$PATH"` : "" return [ "Tests do not run as root in this repo: root reads the files the permission tests make unreadable, so those tests fail here but pass in CI.", @@ -720,6 +730,27 @@ describe("require-nonroot-tests hook", () => { }) }) + it("copies the binary the hook's own node runs from, not the version manager shim found on PATH", () => { + const fixture = createHookFixture() + const nodeBinary = join(fixture.outsideDir, "installs", "node") + const nodeBinaryContent = "#!/bin/sh\n# the node a shim starts\n" + mkdirSync(dirname(nodeBinary)) + writeExecutable(nodeBinary, nodeBinaryContent) + const nobodyNodeDir = nodeCopyFolder(fixture.checkout, MATCHING_NODE_VERSION) + + const run = runHook({ + fixture, + stdin: payloadFor({ command: "npm test", cwd: fixture.checkout }), + nobodyNodeVersion: OTHER_NODE_VERSION, + sessionNodeBinary: nodeBinary, + }) + + expect({ + status: run.status, + copiedNode: readFileSync(join(nobodyNodeDir, "node"), "utf8"), + }).toEqual({ status: 2, copiedNode: nodeBinaryContent }) + }) + it("makes the node copy and its folders readable and runnable by others under a restrictive umask", () => { const fixture = createHookFixture() const nodeCopiesDir = join(fixture.checkout, "node_modules", ".nobody-node") @@ -780,12 +811,13 @@ describe("require-nonroot-tests hook", () => { }) const run = runHook({ fixture, stdin, nobodyNodeVersion: OTHER_NODE_VERSION }) - // The cut-short run fails at its cp, so the second run meets the - // partial file it left. + // The cut-short run fails at its cp, whose partial file was already + // executable, and removes that file. The second run copies again. expect({ cutShortStatus: cutShortRun.status, run, copiedNode: readFileSync(join(nobodyNodeDir, "node"), "utf8"), + copyFolderEntries: readdirSync(nobodyNodeDir), }).toEqual({ cutShortStatus: 1, run: { @@ -794,6 +826,7 @@ describe("require-nonroot-tests hook", () => { stderr: `${expectedRefusal(fixture.checkout, { nobodyNodeDir })}${expectedNodeCopyLine({ nobodyNodeVersion: OTHER_NODE_VERSION, copiedNodeVersion: MATCHING_NODE_VERSION })}`, }, copiedNode: SESSION_NODE_STUB, + copyFolderEntries: ["node"], }) }) @@ -879,6 +912,35 @@ describe("require-nonroot-tests hook", () => { }) }) + it("copies the node in ~/.nvm when its versions/node folder is a symlink", () => { + const fixture = createHookFixture() + const movedNvmDir = join(fixture.outsideDir, "moved-nvm") + const nvmVersionsDir = join(fixture.outsideDir, ".nvm", "versions") + writeNvmNodes(movedNvmDir, ["v24.10.0"]) + mkdirSync(nvmVersionsDir, { recursive: true }) + symlinkSync(join(movedNvmDir, "versions", "node"), join(nvmVersionsDir, "node")) + const nobodyNodeDir = nodeCopyFolder(fixture.checkout, "v24.10.0") + + const run = runHook({ + fixture, + stdin: payloadFor({ command: "npm test", cwd: fixture.checkout }), + nobodyNodeVersion: OTHER_NODE_VERSION, + sessionNodeVersion: OTHER_NODE_VERSION, + }) + + expect({ + run, + copiedNode: readFileSync(join(nobodyNodeDir, "node"), "utf8"), + }).toEqual({ + run: { + status: 2, + stdout: "", + stderr: `${expectedRefusal(fixture.checkout, { nobodyNodeDir })}${expectedNodeCopyLine({ nobodyNodeVersion: OTHER_NODE_VERSION, copiedNodeVersion: "v24.10.0" })}`, + }, + copiedNode: "#!/bin/sh\n# node v24.10.0\n", + }) + }) + it("copies a node of the major version a full .nvmrc version names", () => { const fixture = createHookFixture() writeFileSync(join(fixture.checkout, ".nvmrc"), "v24.1.0\n") From 203b1dfbca8a15057e0e4742ef86700bb2cda2cc Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 20:51:52 +0000 Subject: [PATCH 08/19] fix(hooks): keep a failing nvm search from ending the hook, and name /usr/bin/env in the prefix - find exits non-zero on a folder it cannot read, and under set -e and pipefail that ended the hook with status 1, which lets the run through as root. The search now keeps the nodes it could list. - The printed prefix names /usr/bin/env in full, for the same reason the probe does: setpriv searches PATH with root's capabilities. - The test file's header lists the node stub. Co-Authored-By: Claude Opus 5.5 --- .claude/hooks/require-nonroot-tests.sh | 10 ++++- src/__tests__/require-nonroot-tests.test.ts | 44 ++++++++++++++++++--- 2 files changed, 47 insertions(+), 7 deletions(-) diff --git a/.claude/hooks/require-nonroot-tests.sh b/.claude/hooks/require-nonroot-tests.sh index 2ef4c1563..9545b69fe 100755 --- a/.claude/hooks/require-nonroot-tests.sh +++ b/.claude/hooks/require-nonroot-tests.sh @@ -388,7 +388,10 @@ if [[ "${node_major_differs}" == true ]]; then # named by its version: /versions/node/v24.11.0/bin/node. # sort -V orders v24.9.0 before v24.10.0, so the last line is the newest. # -L enters a versions/node or version folder that is a symlink. - newest_node="$(find -L "${nvm_dir}/versions/node" -mindepth 3 -maxdepth 3 -path "*/v${nvmrc_major}.*/bin/node" 2>/dev/null | sort -V | tail -n 1)" + # A folder find cannot read makes it exit non-zero while still listing + # the nodes it could read, so || true keeps that list rather than let + # set -e end the hook, which would let the run through as root. + newest_node="$(find -L "${nvm_dir}/versions/node" -mindepth 3 -maxdepth 3 -path "*/v${nvmrc_major}.*/bin/node" 2>/dev/null | sort -V | tail -n 1)" || true [[ -n "${newest_node}" ]] || continue nvmrc_node_path="${newest_node}" nvmrc_node_version="$(basename "$(dirname "$(dirname "${newest_node}")")")" @@ -458,10 +461,13 @@ if [[ -n "${nobody_node_dir}" ]]; then env_arguments+=("PATH=${nobody_node_dir_for_shell}:\"\$PATH\"") fi +# The prefix names /usr/bin/env in full for the reason the probe above does: +# setpriv still has root's capabilities when it searches PATH, so a bare env +# could resolve to one nobody's own search would never reach. cat >&2 < { }) }) + it("still copies the newest node it can read when find cannot read a folder under nvm's versions", () => { + const fixture = createHookFixture() + const nvmDir = join(fixture.outsideDir, ".nvm") + writeNvmNodes(nvmDir, ["v24.10.0"]) + // A folder find cannot enter makes it exit non-zero. + const unreadableFolder = join(nvmDir, "versions", "node", "v24.11.0") + mkdirSync(unreadableFolder) + chmodSync(unreadableFolder, 0o000) + onTestFinished(() => chmodSync(unreadableFolder, 0o755)) + const nobodyNodeDir = nodeCopyFolder(fixture.checkout, "v24.10.0") + + const run = runHook({ + fixture, + stdin: payloadFor({ command: "npm test", cwd: fixture.checkout }), + nobodyNodeVersion: OTHER_NODE_VERSION, + sessionNodeVersion: OTHER_NODE_VERSION, + }) + + expect({ + run, + copiedNode: readFileSync(join(nobodyNodeDir, "node"), "utf8"), + }).toEqual({ + run: { + status: 2, + stdout: "", + stderr: `${expectedRefusal(fixture.checkout, { nobodyNodeDir })}${expectedNodeCopyLine({ nobodyNodeVersion: OTHER_NODE_VERSION, copiedNodeVersion: "v24.10.0" })}`, + }, + copiedNode: "#!/bin/sh\n# node v24.10.0\n", + }) + }) + it("copies the node in $NVM_DIR before a newer one in ~/.nvm", () => { const fixture = createHookFixture() const nvmDir = join(fixture.outsideDir, "nvm-dir") From 617157f0171e046ff880fb2e9512b1d4429f4159 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 21:07:00 +0000 Subject: [PATCH 09/19] fix: name the symlink loop in the nvm search's comment, and run the setpriv cases through /usr/bin/env MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Root reads a folder whatever its mode, so the hook, which runs only as root, rarely meets one find cannot read. A symlink loop that -L walks into also makes find exit non-zero while it still lists the other nodes, so the comment on || true names both. - The two test commands that stand for a run through the reply's prefix now use /usr/bin/env, as the reply does. Co-Authored-By: Claude Opus 5.5 Ship-Check: bug-check · claude-opus-5-5 --- .claude/hooks/require-nonroot-tests.sh | 7 ++++--- src/__tests__/require-nonroot-tests.test.ts | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/.claude/hooks/require-nonroot-tests.sh b/.claude/hooks/require-nonroot-tests.sh index 9545b69fe..74c9f5e13 100755 --- a/.claude/hooks/require-nonroot-tests.sh +++ b/.claude/hooks/require-nonroot-tests.sh @@ -388,9 +388,10 @@ if [[ "${node_major_differs}" == true ]]; then # named by its version: /versions/node/v24.11.0/bin/node. # sort -V orders v24.9.0 before v24.10.0, so the last line is the newest. # -L enters a versions/node or version folder that is a symlink. - # A folder find cannot read makes it exit non-zero while still listing - # the nodes it could read, so || true keeps that list rather than let - # set -e end the hook, which would let the run through as root. + # find exits non-zero on a folder it cannot read or a symlink loop that + # -L walks into, while still listing the nodes it reached, so || true + # keeps that list rather than let set -e end the hook, which would let + # the run through as root. newest_node="$(find -L "${nvm_dir}/versions/node" -mindepth 3 -maxdepth 3 -path "*/v${nvmrc_major}.*/bin/node" 2>/dev/null | sort -V | tail -n 1)" || true [[ -n "${newest_node}" ]] || continue nvmrc_node_path="${newest_node}" diff --git a/src/__tests__/require-nonroot-tests.test.ts b/src/__tests__/require-nonroot-tests.test.ts index 1ca4244ff..1991d2d27 100644 --- a/src/__tests__/require-nonroot-tests.test.ts +++ b/src/__tests__/require-nonroot-tests.test.ts @@ -547,7 +547,7 @@ describe("require-nonroot-tests hook", () => { { label: "a test run after a run through setpriv", command: - "setpriv --reuid=65534 --regid=65534 --clear-groups env HOME=/repo/node_modules/.nobody-home npm test; npm test", + "setpriv --reuid=65534 --regid=65534 --clear-groups /usr/bin/env HOME=/repo/node_modules/.nobody-home npm test; npm test", }, ])("stops $label and replies with the nobody prefix", ({ command }) => { const fixture = createHookFixture() @@ -627,7 +627,7 @@ describe("require-nonroot-tests hook", () => { { label: "a command already run through setpriv", command: - "setpriv --reuid=65534 --regid=65534 --clear-groups env HOME=/repo/node_modules/.nobody-home npm test", + "setpriv --reuid=65534 --regid=65534 --clear-groups /usr/bin/env HOME=/repo/node_modules/.nobody-home npm test", }, ])("lets $label through without preparing folders", ({ command }) => { const fixture = createHookFixture() From 231b5998ff3b79f1235a21047d831037a7ea79a9 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 21:09:28 +0000 Subject: [PATCH 10/19] fix(hooks): stop test runs through env, timeout or a shell named by path, and anchor the nvm search - The reply's prefix now names /usr/bin/env, so a run that kept only the env part of that prefix went through as root: the wrapper patterns matched env, time, timeout and the shells only by their bare names. Each may now be named by path. - The nvm search matched a node of another major version when the nvm folder's own path held a folder such as v24.x. find now runs from inside versions/node, so -path sees only nvm's own folders. Co-Authored-By: Claude Opus 5.5 --- .claude/hooks/require-nonroot-tests.sh | 18 +++++++---- src/__tests__/require-nonroot-tests.test.ts | 33 +++++++++++++++++++++ 2 files changed, 45 insertions(+), 6 deletions(-) diff --git a/.claude/hooks/require-nonroot-tests.sh b/.claude/hooks/require-nonroot-tests.sh index 74c9f5e13..a6c8648f1 100755 --- a/.claude/hooks/require-nonroot-tests.sh +++ b/.claude/hooks/require-nonroot-tests.sh @@ -102,12 +102,16 @@ variable_setting="[A-Za-z_][A-Za-z0-9_]*=(${double_quoted}|${single_quoted}|${un # quotes, so a run or a cd after ; or && inside the quoted text counts too # (`bash -c 'npm ci && cd /repo && npm test'`), and a mention there # (`bash -c 'echo npm test'`) does not. +# - Each of env, time, timeout and the shells may be named by path +# (`/usr/bin/env npm test`), as the reply's own prefix names env. exec and +# command are shell builtins, so they have no path. # setpriv is not a wrapper here: a run through setpriv is the nobody run the # reply asks for, so the runner after it never counts. -timeout_wrapper="timeout${flags_with_optional_argument}[[:space:]]+[0-9][^[:space:]]*" +optional_command_folder='([^[:space:]]*/)?' +timeout_wrapper="${optional_command_folder}timeout${flags_with_optional_argument}[[:space:]]+[0-9][^[:space:]]*" shell_flags='([[:space:]]+([-+][[:alpha:]]*[oO][[:alpha:]]*[[:space:]]+[^[:space:]]+|[-+][^[:space:]]*))*' -shell_wrapper="(bash|sh|zsh)${shell_flags}[[:space:]]+([\"'][[:space:]]*)?" -wrapper="((env|time)${flags_without_argument}|exec|command|${timeout_wrapper})[[:space:]]+|${shell_wrapper}" +shell_wrapper="${optional_command_folder}(bash|sh|zsh)${shell_flags}[[:space:]]+([\"'][[:space:]]*)?" +wrapper="(${optional_command_folder}(env|time)${flags_without_argument}|exec|command|${timeout_wrapper})[[:space:]]+|${shell_wrapper}" # shell_keyword matches a shell keyword a command can follow: if # (`if npm test`), then, do, else, elif, while, until, ! (`! npm test`) and { @@ -386,16 +390,18 @@ if [[ "${node_major_differs}" == true ]]; then [[ -n "${nvm_dir}" && -d "${nvm_dir}/versions/node" ]] || continue # nvm keeps each node three folders below versions/node, in a folder # named by its version: /versions/node/v24.11.0/bin/node. + # find runs from inside versions/node, so -path sees only those three + # folders: a v24. elsewhere in the nvm folder's own path cannot match. # sort -V orders v24.9.0 before v24.10.0, so the last line is the newest. # -L enters a versions/node or version folder that is a symlink. # find exits non-zero on a folder it cannot read or a symlink loop that # -L walks into, while still listing the nodes it reached, so || true # keeps that list rather than let set -e end the hook, which would let # the run through as root. - newest_node="$(find -L "${nvm_dir}/versions/node" -mindepth 3 -maxdepth 3 -path "*/v${nvmrc_major}.*/bin/node" 2>/dev/null | sort -V | tail -n 1)" || true + newest_node="$(cd "${nvm_dir}/versions/node" && find -L . -mindepth 3 -maxdepth 3 -path "./v${nvmrc_major}.*/bin/node" 2>/dev/null | sort -V | tail -n 1)" || true [[ -n "${newest_node}" ]] || continue - nvmrc_node_path="${newest_node}" - nvmrc_node_version="$(basename "$(dirname "$(dirname "${newest_node}")")")" + nvmrc_node_path="${nvm_dir}/versions/node/${newest_node#./}" + nvmrc_node_version="$(basename "$(dirname "$(dirname "${nvmrc_node_path}")")")" break done fi diff --git a/src/__tests__/require-nonroot-tests.test.ts b/src/__tests__/require-nonroot-tests.test.ts index 1991d2d27..f20596a4f 100644 --- a/src/__tests__/require-nonroot-tests.test.ts +++ b/src/__tests__/require-nonroot-tests.test.ts @@ -508,6 +508,12 @@ describe("require-nonroot-tests hook", () => { { label: "a variable setting before npx", command: "CI=1 npx vitest run" }, { label: "env with a variable setting", command: "env CI=1 npm test" }, { label: "env with a flag", command: "env -i npm test" }, + { + label: "env by its path, as the reply's prefix names it", + command: "/usr/bin/env HOME=/repo/node_modules/.nobody-home npm test", + }, + { label: "timeout by its path", command: "/usr/bin/timeout 600 npm test" }, + { label: "a shell by its path", command: "/bin/bash -c 'npm test'" }, { label: "a double-quoted value with a space", command: 'FOO="a b" npm test' }, { label: "time", command: "time npm test" }, { label: "time with a flag", command: "time -p npm test" }, @@ -890,6 +896,33 @@ describe("require-nonroot-tests hook", () => { }) }) + it("matches the .nvmrc major only in the version folder's name, not in the nvm folder's own path", () => { + const fixture = createHookFixture() + // The nvm folder's own path holds /v24.x/, and its only node is v18. + const nvmDir = join(fixture.outsideDir, "v24.x", "nvm") + writeNvmNodes(nvmDir, ["v18.20.0"]) + + const run = runHook({ + fixture, + stdin: payloadFor({ command: "npm test", cwd: fixture.checkout }), + nobodyNodeVersion: OTHER_NODE_VERSION, + sessionNodeVersion: OTHER_NODE_VERSION, + nvmDir, + }) + + expect({ + run, + nodeCopiesMade: existsSync(join(fixture.checkout, "node_modules", ".nobody-node")), + }).toEqual({ + run: { + status: 2, + stdout: "", + stderr: `${expectedRefusal(fixture.checkout)}nobody's node is ${OTHER_NODE_VERSION}, not the 24 that .nvmrc names and CI runs, so a result can differ from CI's.\n`, + }, + nodeCopiesMade: false, + }) + }) + it("copies the node in $NVM_DIR before a newer one in ~/.nvm", () => { const fixture = createHookFixture() const nvmDir = join(fixture.outsideDir, "nvm-dir") From e8ca7ddbb92c3c3d95705c8edabe80629d50f1f7 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 21:23:10 +0000 Subject: [PATCH 11/19] fix(hooks): stop the reply's full prefix run without setpriv, and quiet the nvm search's cd MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - In a cloud container the reply's prefix carries env's -u NODE_EXTRA_CA_CERTS and a PATH setting of the form /copy:"$PATH". The wrapper read -u's name as the command env runs, and a variable setting took only one quoting form for its whole value, so that prefix used without setpriv still went through as root. env's -u and --unset now take their name, and a value may mix unquoted and quoted parts. - The nvm search's cd printed its error into the reply when $NVM_DIR's versions/node passed the -d test but could not be entered, where find had dropped it. cd's error now goes to /dev/null, and CDPATH is cleared so a relative $NVM_DIR enters the folder the -d test checked. Co-Authored-By: Claude Opus 5.5 Ship-Check: bug-check · claude-opus-5-5 --- .claude/hooks/require-nonroot-tests.sh | 17 +++++--- src/__tests__/require-nonroot-tests.test.ts | 45 +++++++++++++++++++++ 2 files changed, 57 insertions(+), 5 deletions(-) diff --git a/.claude/hooks/require-nonroot-tests.sh b/.claude/hooks/require-nonroot-tests.sh index a6c8648f1..8165d2c12 100755 --- a/.claude/hooks/require-nonroot-tests.sh +++ b/.claude/hooks/require-nonroot-tests.sh @@ -80,18 +80,22 @@ npm_runner="npm${flags_with_optional_argument}[[:space:]]+(test|t|run(-script)?$ test_runner="${vitest_runner}|${npm_exec_runner}|${npm_runner}" # variable_setting matches a VAR=value setting before the command -# (`CI=1 npm test`), with the value in one of three forms. +# (`CI=1 npm test`). The value is unquoted text with any number of quoted +# parts, each followed by more unquoted text, as in the reply's own prefix +# (`PATH=/copy:"$PATH"`). # - double_quoted matches text in double quotes, with no " inside. # - single_quoted matches text in single quotes, with no ' inside. # - unquoted matches text up to the first space, ' or ". double_quoted='"[^"]*"' single_quoted="'[^']*'" unquoted="[^[:space:]'\"]*" -variable_setting="[A-Za-z_][A-Za-z0-9_]*=(${double_quoted}|${single_quoted}|${unquoted})[[:space:]]+" +variable_setting="[A-Za-z_][A-Za-z0-9_]*=${unquoted}((${double_quoted}|${single_quoted})${unquoted})*[[:space:]]+" # wrapper matches these commands, which run the command after them: # - env and time, with flags that take no argument, because a flag's argument -# would look the same as the command it runs. +# would look the same as the command it runs. env's -u (--unset) is the +# exception: it always takes a name, as in the reply's own prefix in a cloud +# container (`env -u NODE_EXTRA_CA_CERTS npm test`). # - exec and command, without flags, so `command -v vitest` is not a run. # - timeout, whose flags may each take one argument, then the duration # (`timeout -s KILL 600 npm test`, `timeout -k 5 600 npm test`). @@ -108,10 +112,11 @@ variable_setting="[A-Za-z_][A-Za-z0-9_]*=(${double_quoted}|${single_quoted}|${un # setpriv is not a wrapper here: a run through setpriv is the nobody run the # reply asks for, so the runner after it never counts. optional_command_folder='([^[:space:]]*/)?' +env_flags='([[:space:]]+((-u|--unset)[[:space:]]+[^[:space:]]+|-[^[:space:]]*))*' timeout_wrapper="${optional_command_folder}timeout${flags_with_optional_argument}[[:space:]]+[0-9][^[:space:]]*" shell_flags='([[:space:]]+([-+][[:alpha:]]*[oO][[:alpha:]]*[[:space:]]+[^[:space:]]+|[-+][^[:space:]]*))*' shell_wrapper="${optional_command_folder}(bash|sh|zsh)${shell_flags}[[:space:]]+([\"'][[:space:]]*)?" -wrapper="(${optional_command_folder}(env|time)${flags_without_argument}|exec|command|${timeout_wrapper})[[:space:]]+|${shell_wrapper}" +wrapper="(${optional_command_folder}env${env_flags}|${optional_command_folder}time${flags_without_argument}|exec|command|${timeout_wrapper})[[:space:]]+|${shell_wrapper}" # shell_keyword matches a shell keyword a command can follow: if # (`if npm test`), then, do, else, elif, while, until, ! (`! npm test`) and { @@ -392,13 +397,15 @@ if [[ "${node_major_differs}" == true ]]; then # named by its version: /versions/node/v24.11.0/bin/node. # find runs from inside versions/node, so -path sees only those three # folders: a v24. elsewhere in the nvm folder's own path cannot match. + # CDPATH is cleared so cd enters the folder the -d test checked, and + # cd's error on a folder it cannot enter is dropped, as find's is. # sort -V orders v24.9.0 before v24.10.0, so the last line is the newest. # -L enters a versions/node or version folder that is a symlink. # find exits non-zero on a folder it cannot read or a symlink loop that # -L walks into, while still listing the nodes it reached, so || true # keeps that list rather than let set -e end the hook, which would let # the run through as root. - newest_node="$(cd "${nvm_dir}/versions/node" && find -L . -mindepth 3 -maxdepth 3 -path "./v${nvmrc_major}.*/bin/node" 2>/dev/null | sort -V | tail -n 1)" || true + newest_node="$(CDPATH='' cd "${nvm_dir}/versions/node" 2>/dev/null && find -L . -mindepth 3 -maxdepth 3 -path "./v${nvmrc_major}.*/bin/node" 2>/dev/null | sort -V | tail -n 1)" || true [[ -n "${newest_node}" ]] || continue nvmrc_node_path="${nvm_dir}/versions/node/${newest_node#./}" nvmrc_node_version="$(basename "$(dirname "$(dirname "${nvmrc_node_path}")")")" diff --git a/src/__tests__/require-nonroot-tests.test.ts b/src/__tests__/require-nonroot-tests.test.ts index f20596a4f..651f19062 100644 --- a/src/__tests__/require-nonroot-tests.test.ts +++ b/src/__tests__/require-nonroot-tests.test.ts @@ -514,7 +514,20 @@ describe("require-nonroot-tests hook", () => { }, { label: "timeout by its path", command: "/usr/bin/timeout 600 npm test" }, { label: "a shell by its path", command: "/bin/bash -c 'npm test'" }, + { + label: "env with -u and the name it unsets", + command: "env -u NODE_EXTRA_CA_CERTS npm test", + }, + { + label: "the reply's full prefix without setpriv", + command: + '/usr/bin/env -u NODE_EXTRA_CA_CERTS HOME=/repo/node_modules/.nobody-home PATH=/repo/node_modules/.nobody-node/node-v24.11.0:"$PATH" npm test', + }, { label: "a double-quoted value with a space", command: 'FOO="a b" npm test' }, + { + label: "a value that mixes unquoted and quoted text", + command: 'PATH=/copy:"$PATH" npm test', + }, { label: "time", command: "time npm test" }, { label: "time with a flag", command: "time -p npm test" }, { @@ -979,6 +992,38 @@ describe("require-nonroot-tests hook", () => { }) }) + it("copies the node in ~/.nvm, adding nothing to the reply, when $NVM_DIR's versions/node folder cannot be entered", () => { + const fixture = createHookFixture() + const nvmDir = join(fixture.outsideDir, "nvm-dir") + writeNvmNodes(nvmDir, ["v24.1.0"]) + // Readable but not searchable: the -d test passes, and cd fails. + const lockedVersionsDir = join(nvmDir, "versions", "node") + chmodSync(lockedVersionsDir, 0o600) + onTestFinished(() => chmodSync(lockedVersionsDir, 0o755)) + writeNvmNodes(join(fixture.outsideDir, ".nvm"), ["v24.10.0"]) + const nobodyNodeDir = nodeCopyFolder(fixture.checkout, "v24.10.0") + + const run = runHook({ + fixture, + stdin: payloadFor({ command: "npm test", cwd: fixture.checkout }), + nobodyNodeVersion: OTHER_NODE_VERSION, + sessionNodeVersion: OTHER_NODE_VERSION, + nvmDir, + }) + + expect({ + run, + copiedNode: readFileSync(join(nobodyNodeDir, "node"), "utf8"), + }).toEqual({ + run: { + status: 2, + stdout: "", + stderr: `${expectedRefusal(fixture.checkout, { nobodyNodeDir })}${expectedNodeCopyLine({ nobodyNodeVersion: OTHER_NODE_VERSION, copiedNodeVersion: "v24.10.0" })}`, + }, + copiedNode: "#!/bin/sh\n# node v24.10.0\n", + }) + }) + it("copies the node in ~/.nvm when its versions/node folder is a symlink", () => { const fixture = createHookFixture() const movedNvmDir = join(fixture.outsideDir, "moved-nvm") From c8874b81ebaf879300e40f90d304a09419964c04 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 22:09:25 +0000 Subject: [PATCH 12/19] fix(hooks): stop the reply's prefix with an escaped space, and env's -C and grouped -u - In a checkout whose path holds a space, the reply's prefix writes the space escaped (`HOME=/my\ repo/...`). The unquoted part of a setting stopped at that space, so the prefix without setpriv ran as root. A backslash now escapes the next character. - env's -C (--chdir) takes a folder, and -u also takes a name at the end of a group of flags (`env -iu NAME`). Both read their argument as the command, so the run after them went through. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01PkA1FHrosoEim4HpZyQGyQ --- .claude/hooks/require-nonroot-tests.sh | 16 ++++++++++------ src/__tests__/require-nonroot-tests.test.ts | 10 ++++++++++ 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/.claude/hooks/require-nonroot-tests.sh b/.claude/hooks/require-nonroot-tests.sh index 8165d2c12..88e9b4a58 100755 --- a/.claude/hooks/require-nonroot-tests.sh +++ b/.claude/hooks/require-nonroot-tests.sh @@ -85,17 +85,21 @@ test_runner="${vitest_runner}|${npm_exec_runner}|${npm_runner}" # (`PATH=/copy:"$PATH"`). # - double_quoted matches text in double quotes, with no " inside. # - single_quoted matches text in single quotes, with no ' inside. -# - unquoted matches text up to the first space, ' or ". +# - unquoted matches text up to the first space, ' or " that no backslash +# escapes. The reply's prefix escapes a space in the checkout's path +# (`HOME=/my\ repo/node_modules/.nobody-home`). double_quoted='"[^"]*"' single_quoted="'[^']*'" -unquoted="[^[:space:]'\"]*" +unquoted="([^[:space:]'\"\\\\]|\\\\.)*" variable_setting="[A-Za-z_][A-Za-z0-9_]*=${unquoted}((${double_quoted}|${single_quoted})${unquoted})*[[:space:]]+" # wrapper matches these commands, which run the command after them: # - env and time, with flags that take no argument, because a flag's argument -# would look the same as the command it runs. env's -u (--unset) is the -# exception: it always takes a name, as in the reply's own prefix in a cloud -# container (`env -u NODE_EXTRA_CA_CERTS npm test`). +# would look the same as the command it runs. env's -u (--unset) and -C +# (--chdir) are the exceptions: each always takes an argument, also when it +# ends a group of one-letter flags (`env -iu NAME npm test`). The reply's +# own prefix in a cloud container uses -u +# (`env -u NODE_EXTRA_CA_CERTS npm test`). # - exec and command, without flags, so `command -v vitest` is not a run. # - timeout, whose flags may each take one argument, then the duration # (`timeout -s KILL 600 npm test`, `timeout -k 5 600 npm test`). @@ -112,7 +116,7 @@ variable_setting="[A-Za-z_][A-Za-z0-9_]*=${unquoted}((${double_quoted}|${single_ # setpriv is not a wrapper here: a run through setpriv is the nobody run the # reply asks for, so the runner after it never counts. optional_command_folder='([^[:space:]]*/)?' -env_flags='([[:space:]]+((-u|--unset)[[:space:]]+[^[:space:]]+|-[^[:space:]]*))*' +env_flags='([[:space:]]+((-[[:alnum:]]*[uC]|--unset|--chdir)[[:space:]]+[^[:space:]]+|-[^[:space:]]*))*' timeout_wrapper="${optional_command_folder}timeout${flags_with_optional_argument}[[:space:]]+[0-9][^[:space:]]*" shell_flags='([[:space:]]+([-+][[:alpha:]]*[oO][[:alpha:]]*[[:space:]]+[^[:space:]]+|[-+][^[:space:]]*))*' shell_wrapper="${optional_command_folder}(bash|sh|zsh)${shell_flags}[[:space:]]+([\"'][[:space:]]*)?" diff --git a/src/__tests__/require-nonroot-tests.test.ts b/src/__tests__/require-nonroot-tests.test.ts index 651f19062..b69101ce1 100644 --- a/src/__tests__/require-nonroot-tests.test.ts +++ b/src/__tests__/require-nonroot-tests.test.ts @@ -523,6 +523,14 @@ describe("require-nonroot-tests hook", () => { command: '/usr/bin/env -u NODE_EXTRA_CA_CERTS HOME=/repo/node_modules/.nobody-home PATH=/repo/node_modules/.nobody-node/node-v24.11.0:"$PATH" npm test', }, + { + label: "the reply's full prefix without setpriv, in a checkout whose path has a space", + command: + '/usr/bin/env -u NODE_EXTRA_CA_CERTS HOME=/my\\ repo/node_modules/.nobody-home PATH=/my\\ repo/node_modules/.nobody-node/node-v24.11.0:"$PATH" npm test', + }, + { label: "env with -u at the end of a group of flags", command: "env -iu FOO npm test" }, + { label: "env with -C and the folder it enters", command: "env -C /tmp npm test" }, + { label: "env with --chdir and the folder it enters", command: "env --chdir /tmp npm test" }, { label: "a double-quoted value with a space", command: 'FOO="a b" npm test' }, { label: "a value that mixes unquoted and quoted text", @@ -589,6 +597,8 @@ describe("require-nonroot-tests hook", () => { { label: "npm run-script of another script", command: "npm run-script lint" }, { label: "npm exec of another binary", command: "npm exec prettier -- --version" }, { label: "bash -c with a quoted non-test command", command: "bash -c 'npm run lint'" }, + // The escaped space belongs to A's value, so the command is test, not npm. + { label: "a setting whose value ends in an escaped space", command: "A=a\\ npm test" }, { label: "a quoted grep pattern that a | splits after a runner name", command: "grep -E 'npm test|vitest' src/", From 9008ad1ef723a05af0040238803d93fc27dc9a86 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 22:14:34 +0000 Subject: [PATCH 13/19] test(hooks): pin the cloud reply with both the CA unset and the node copy No test set both parts of the prefix at once, though a cloud container's reply has both. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01PkA1FHrosoEim4HpZyQGyQ --- src/__tests__/require-nonroot-tests.test.ts | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/__tests__/require-nonroot-tests.test.ts b/src/__tests__/require-nonroot-tests.test.ts index b69101ce1..825e68f13 100644 --- a/src/__tests__/require-nonroot-tests.test.ts +++ b/src/__tests__/require-nonroot-tests.test.ts @@ -1144,6 +1144,26 @@ describe("require-nonroot-tests hook", () => { }) }) + // The reply a cloud container gets: env's -u must come before the + // settings, or env would take -u as the command to run. + it("drops NODE_EXTRA_CA_CERTS ahead of the settings when the prefix also runs a node copy", () => { + const fixture = createHookFixture() + const nobodyNodeDir = nodeCopyFolder(fixture.checkout, MATCHING_NODE_VERSION) + + const run = runHook({ + fixture, + stdin: payloadFor({ command: "npm test", cwd: fixture.checkout }), + nobodyNodeVersion: OTHER_NODE_VERSION, + extraCaCerts: join(fixture.outsideDir, "missing-ca-bundle.crt"), + }) + + expect(run).toEqual({ + status: 2, + stdout: "", + stderr: `${expectedRefusal(fixture.checkout, { unsetsExtraCaCerts: true, nobodyNodeDir })}${expectedNodeCopyLine({ nobodyNodeVersion: OTHER_NODE_VERSION, copiedNodeVersion: MATCHING_NODE_VERSION })}`, + }) + }) + it("keeps NODE_EXTRA_CA_CERTS when nobody can read its file", () => { const fixture = createHookFixture() const caBundle = join(fixture.outsideDir, "ca-bundle.crt") From 592cca19db86a9ef7101e2651f7d6de184740da6 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 22:28:13 +0000 Subject: [PATCH 14/19] fix(hooks): follow a cd whose path escapes a space An unquoted cd path stopped at its first space, escaped or not, so `cd /my\ repo && npm test` named /my\ and, from a folder in no checkout, let the run through as root. The cd patterns now take a backslash-escaped character, and the walk drops the backslash as the shell does. Co-Authored-By: Claude Opus 5.5 --- .claude/hooks/require-nonroot-tests.sh | 28 ++++++++++++++---- src/__tests__/require-nonroot-tests.test.ts | 32 +++++++++++++++++++++ 2 files changed, 55 insertions(+), 5 deletions(-) diff --git a/.claude/hooks/require-nonroot-tests.sh b/.claude/hooks/require-nonroot-tests.sh index 88e9b4a58..f0e02a4b2 100755 --- a/.claude/hooks/require-nonroot-tests.sh +++ b/.claude/hooks/require-nonroot-tests.sh @@ -171,7 +171,9 @@ exempt_suite_run="${vitest_exempt_suite_run}|${npm_exempt_suite_run}" # test'`) is cut off first, with leading_command_prefix, so the first three # patterns start at cd or pushd and the path is the second group of their # match. any_cd accepts that prefix itself for the grep below. -# - cd_to_absolute_path matches an unquoted path that starts with /. +# - cd_to_absolute_path matches an unquoted path that starts with /. In it, +# and in cd_to_relative_path, a backslash escapes the next character +# (`cd /my\ repo`), and the walk drops the backslash. # - cd_to_quoted_absolute_path matches a quoted path that starts with / and # holds no $, backtick or ~, so the shell would use it as written. # - cd_to_relative_path matches an unquoted relative path. Its first @@ -181,10 +183,26 @@ exempt_suite_run="${vitest_exempt_suite_run}|${npm_exempt_suite_run}" # catches the rest: a bare `cd`, with or without a comment after it, # `cd -`, popd, and a ~, $ or quoted path the others do not take. directory_change="^[[:space:]]*(cd|pushd)[[:space:]]+" -cd_to_absolute_path="${directory_change}(/[^[:space:]]*)" +escaped_path_character='([^[:space:]\\]|\\.)' +cd_to_absolute_path="${directory_change}(/${escaped_path_character}*)" cd_to_quoted_absolute_path="${directory_change}[\"'](/[^\"'\$\`~]*)[\"']" -cd_to_relative_path="${directory_change}([^-~#\"'\$[:space:]][^[:space:]]*)" +cd_to_relative_path="${directory_change}(([^-~#\"'\$[:space:]\\\\]|\\\\.)${escaped_path_character}*)" leading_command_prefix="^[[:space:]]*${command_prefix}" + +# The shell drops the backslash in front of an escaped character in an +# unquoted path, so the walk does the same to a path it takes from a cd. +# Bash, not sed: the hook runs no tool it does not need. +backslash_escape='^([^\\]*)\\(.)(.*)$' +remove_backslash_escapes() { + local remaining_path="$1" + local unescaped_path="" + + while [[ "${remaining_path}" =~ ${backslash_escape} ]]; do + unescaped_path+="${BASH_REMATCH[1]}${BASH_REMATCH[2]}" + remaining_path="${BASH_REMATCH[3]}" + done + printf '%s' "${unescaped_path}${remaining_path}" +} any_cd="^[[:space:]]*${command_prefix}(cd|pushd|popd)([[:space:]]|$)" # The command is split into the commands it runs, one per line, and only the @@ -246,7 +264,7 @@ while IFS= read -r split_command; do cd_candidate="${split_command:${#BASH_REMATCH[0]}}" fi if [[ "${cd_candidate}" =~ ${cd_to_absolute_path} ]]; then - current_dir="${BASH_REMATCH[2]}" + current_dir="$(remove_backslash_escapes "${BASH_REMATCH[2]}")" continue fi if [[ "${cd_candidate}" =~ ${cd_to_quoted_absolute_path} ]]; then @@ -254,7 +272,7 @@ while IFS= read -r split_command; do continue fi if [[ "${cd_candidate}" =~ ${cd_to_relative_path} ]]; then - current_dir="${current_dir}/${BASH_REMATCH[2]}" + current_dir="${current_dir}/$(remove_backslash_escapes "${BASH_REMATCH[2]}")" continue fi if [[ "${cd_candidate}" =~ ${any_cd} ]]; then diff --git a/src/__tests__/require-nonroot-tests.test.ts b/src/__tests__/require-nonroot-tests.test.ts index 825e68f13..ecbb528d3 100644 --- a/src/__tests__/require-nonroot-tests.test.ts +++ b/src/__tests__/require-nonroot-tests.test.ts @@ -1441,6 +1441,38 @@ describe("require-nonroot-tests hook", () => { expect(run).toEqual({ status: 2, stdout: "", stderr: expectedRefusal(fixture.checkout) }) }) + it.each([ + { + label: "an absolute path", + command: (spacedCheckout: string) => + `cd ${spacedCheckout.replaceAll(" ", "\\ ")} && npm test`, + }, + { + label: "a relative path from an earlier cd", + command: (spacedCheckout: string) => { + return `cd ${dirname(spacedCheckout)} && cd ${basename(spacedCheckout).replaceAll(" ", "\\ ")} && npm test` + }, + }, + ])("prepares the checkout a cd names with an escaped space in $label", ({ command }) => { + const fixture = createHookFixture() + const spacedCheckout = createOtherCheckout(fixture, "spaced checkout") + + // The session's folder is in no checkout, so only the cd's path can + // reach one. + const run = runHook({ + fixture, + stdin: payloadFor({ command: command(spacedCheckout), cwd: fixture.outsideDir }), + }) + + expect(run).toEqual({ + status: 2, + stdout: "", + stderr: expectedRefusal(spacedCheckout, { + nobodyHomeInPrefix: `${spacedCheckout.replaceAll(" ", "\\ ")}/node_modules/.nobody-home`, + }), + }) + }) + it.each([ { label: "a ~ path", target: "~" }, { label: "a variable", target: "$HOME" }, From ce5174034e7079c0f69386cd728756fcee3d3f6c Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 22:30:14 +0000 Subject: [PATCH 15/19] chore(hooks): read env's -u and -C arguments as the shell does - Only env's flags that take no argument (-0, -i, -v) may come before a -u or -C in one group. A u or C earlier in the group takes the rest of the group as its argument, so the next word is the command env runs, and the hook stopped such runs although they run no tests. - A -u or -C argument is now a shell word, the pattern a setting's value uses, so a quoted or escaped space in a -C folder no longer lets the run through. - The walk does not follow env's -C folder, as it does not follow npm's --prefix; the comment now says so. Co-Authored-By: Claude Opus 5.5 --- .claude/hooks/require-nonroot-tests.sh | 24 +++++++++++---------- src/__tests__/require-nonroot-tests.test.ts | 12 +++++++++++ 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/.claude/hooks/require-nonroot-tests.sh b/.claude/hooks/require-nonroot-tests.sh index f0e02a4b2..d2263c77d 100755 --- a/.claude/hooks/require-nonroot-tests.sh +++ b/.claude/hooks/require-nonroot-tests.sh @@ -80,9 +80,9 @@ npm_runner="npm${flags_with_optional_argument}[[:space:]]+(test|t|run(-script)?$ test_runner="${vitest_runner}|${npm_exec_runner}|${npm_runner}" # variable_setting matches a VAR=value setting before the command -# (`CI=1 npm test`). The value is unquoted text with any number of quoted -# parts, each followed by more unquoted text, as in the reply's own prefix -# (`PATH=/copy:"$PATH"`). +# (`CI=1 npm test`). Its value is a shell_word: unquoted text with any number +# of quoted parts, each followed by more unquoted text, as in the reply's own +# prefix (`PATH=/copy:"$PATH"`). # - double_quoted matches text in double quotes, with no " inside. # - single_quoted matches text in single quotes, with no ' inside. # - unquoted matches text up to the first space, ' or " that no backslash @@ -91,15 +91,16 @@ test_runner="${vitest_runner}|${npm_exec_runner}|${npm_runner}" double_quoted='"[^"]*"' single_quoted="'[^']*'" unquoted="([^[:space:]'\"\\\\]|\\\\.)*" -variable_setting="[A-Za-z_][A-Za-z0-9_]*=${unquoted}((${double_quoted}|${single_quoted})${unquoted})*[[:space:]]+" +shell_word="${unquoted}((${double_quoted}|${single_quoted})${unquoted})*" +variable_setting="[A-Za-z_][A-Za-z0-9_]*=${shell_word}[[:space:]]+" # wrapper matches these commands, which run the command after them: # - env and time, with flags that take no argument, because a flag's argument # would look the same as the command it runs. env's -u (--unset) and -C -# (--chdir) are the exceptions: each always takes an argument, also when it -# ends a group of one-letter flags (`env -iu NAME npm test`). The reply's -# own prefix in a cloud container uses -u -# (`env -u NODE_EXTRA_CA_CERTS npm test`). +# (--chdir) are the exceptions: each always takes a shell_word, also when it +# ends a group of env's flags that take none, -0, -i and -v +# (`env -iu NAME npm test`). The reply's own prefix in a cloud container +# uses -u (`env -u NODE_EXTRA_CA_CERTS npm test`). # - exec and command, without flags, so `command -v vitest` is not a run. # - timeout, whose flags may each take one argument, then the duration # (`timeout -s KILL 600 npm test`, `timeout -k 5 600 npm test`). @@ -116,7 +117,7 @@ variable_setting="[A-Za-z_][A-Za-z0-9_]*=${unquoted}((${double_quoted}|${single_ # setpriv is not a wrapper here: a run through setpriv is the nobody run the # reply asks for, so the runner after it never counts. optional_command_folder='([^[:space:]]*/)?' -env_flags='([[:space:]]+((-[[:alnum:]]*[uC]|--unset|--chdir)[[:space:]]+[^[:space:]]+|-[^[:space:]]*))*' +env_flags="([[:space:]]+((-[0iv]*[uC]|--unset|--chdir)[[:space:]]+${shell_word}|-[^[:space:]]*))*" timeout_wrapper="${optional_command_folder}timeout${flags_with_optional_argument}[[:space:]]+[0-9][^[:space:]]*" shell_flags='([[:space:]]+([-+][[:alpha:]]*[oO][[:alpha:]]*[[:space:]]+[^[:space:]]+|[-+][^[:space:]]*))*' shell_wrapper="${optional_command_folder}(bash|sh|zsh)${shell_flags}[[:space:]]+([\"'][[:space:]]*)?" @@ -247,8 +248,9 @@ cd_or_test_commands="$(printf '%s\n' "${tool_command}" | # resolve. A cd after the run does not count. # - A cd after env, exec or timeout moves it too, though the shell's folder # stays put: those commands run a program, and cd is a shell builtin. -# - npm's --prefix folder is not followed: `npm --prefix /other/checkout -# test` counts as `npm test`. +# - npm's --prefix folder and env's -C folder are not followed: +# `npm --prefix /other/checkout test` and `env -C /other/checkout npm +# test` count as `npm test`. session_dir="${payload_cwd:-${CLAUDE_PROJECT_DIR:-.}}" run_dir="${session_dir}" current_dir="${session_dir}" diff --git a/src/__tests__/require-nonroot-tests.test.ts b/src/__tests__/require-nonroot-tests.test.ts index ecbb528d3..2ba5808bc 100644 --- a/src/__tests__/require-nonroot-tests.test.ts +++ b/src/__tests__/require-nonroot-tests.test.ts @@ -531,6 +531,14 @@ describe("require-nonroot-tests hook", () => { { label: "env with -u at the end of a group of flags", command: "env -iu FOO npm test" }, { label: "env with -C and the folder it enters", command: "env -C /tmp npm test" }, { label: "env with --chdir and the folder it enters", command: "env --chdir /tmp npm test" }, + { + label: "env with -C and a double-quoted folder holding a space", + command: 'env -C "/my dir" npm test', + }, + { + label: "env with -C and a folder holding an escaped space", + command: "env -C /my\\ dir npm test", + }, { label: "a double-quoted value with a space", command: 'FOO="a b" npm test' }, { label: "a value that mixes unquoted and quoted text", @@ -599,6 +607,10 @@ describe("require-nonroot-tests hook", () => { { label: "bash -c with a quoted non-test command", command: "bash -c 'npm run lint'" }, // The escaped space belongs to A's value, so the command is test, not npm. { label: "a setting whose value ends in an escaped space", command: "A=a\\ npm test" }, + // In a group, a u or C takes the rest of the group as its argument, so + // FOO is the command env runs. + { label: "env with -u unsetting u, then FOO", command: "env -uu FOO npm test" }, + { label: "env with -u unsetting C, then FOO", command: "env -uC FOO npm test" }, { label: "a quoted grep pattern that a | splits after a runner name", command: "grep -E 'npm test|vitest' src/", From 1c48b78c9afea1bbaf2622bd8e4a7a4e4067d337 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 23:07:42 +0000 Subject: [PATCH 16/19] chore(hooks): keep the cd walk fast, and stop npm by path and escaped quotes - The walk unescaped each cd path in a subshell, with a loop that rescanned the path per escape, so a command with thousands of cds could pass the hook's 10-second limit and run as root. read without -r drops the backslashes as a builtin. - An escaped leading - in a relative cd path (`cd \-foo`) is a flag to cd, as the comment says, so the walk no longer follows it. - A double-quoted part may hold an escaped character, so an env -u or -C argument or a setting's value holding \" is read whole. - npm and npx named by path (/usr/local/bin/npm test) are runners too. - The ~/.nvm search reads HOME with a default, so an unset HOME no longer ends the hook under set -u. - A test pins that env's -C folder is not followed. Co-Authored-By: Claude Opus 5.5 --- .claude/hooks/require-nonroot-tests.sh | 45 ++++++++----------- src/__tests__/require-nonroot-tests.test.ts | 50 ++++++++++++++++++++- 2 files changed, 68 insertions(+), 27 deletions(-) diff --git a/.claude/hooks/require-nonroot-tests.sh b/.claude/hooks/require-nonroot-tests.sh index d2263c77d..dcf73e4c0 100755 --- a/.claude/hooks/require-nonroot-tests.sh +++ b/.claude/hooks/require-nonroot-tests.sh @@ -73,22 +73,26 @@ flags_with_optional_argument='([[:space:]]+-[^[:space:]]*([[:space:]]+[^-[:space # out test:remote-boot and test:cli-pty, the two suites exempt from this # hook because they cannot run as nobody; the exemptions below give each # reason. +# npm and npx may be named by path (`/usr/local/bin/npm test`), as vitest and +# the wrappers below may. +optional_command_folder='([^[:space:]]*/)?' vitest_binary='([^[:space:]]*/)?vitest(@[^[:space:]]*)?' -vitest_runner="(npx${flags_with_optional_argument}[[:space:]]+)?${vitest_binary}|node[[:space:]]+[^[:space:]]*vitest\.mjs" -npm_exec_runner="npm${flags_with_optional_argument}[[:space:]]+(exec|x)${flags_with_optional_argument}[[:space:]]+${vitest_binary}" -npm_runner="npm${flags_with_optional_argument}[[:space:]]+(test|t|run(-script)?${flags_with_optional_argument}[[:space:]]+(test|test:coverage|test:watch|snapshot:update))" +vitest_runner="(${optional_command_folder}npx${flags_with_optional_argument}[[:space:]]+)?${vitest_binary}|node[[:space:]]+[^[:space:]]*vitest\.mjs" +npm_exec_runner="${optional_command_folder}npm${flags_with_optional_argument}[[:space:]]+(exec|x)${flags_with_optional_argument}[[:space:]]+${vitest_binary}" +npm_runner="${optional_command_folder}npm${flags_with_optional_argument}[[:space:]]+(test|t|run(-script)?${flags_with_optional_argument}[[:space:]]+(test|test:coverage|test:watch|snapshot:update))" test_runner="${vitest_runner}|${npm_exec_runner}|${npm_runner}" # variable_setting matches a VAR=value setting before the command # (`CI=1 npm test`). Its value is a shell_word: unquoted text with any number # of quoted parts, each followed by more unquoted text, as in the reply's own # prefix (`PATH=/copy:"$PATH"`). -# - double_quoted matches text in double quotes, with no " inside. +# - double_quoted matches text in double quotes, where a backslash escapes +# the next character (`"a\"b"`). # - single_quoted matches text in single quotes, with no ' inside. # - unquoted matches text up to the first space, ' or " that no backslash # escapes. The reply's prefix escapes a space in the checkout's path # (`HOME=/my\ repo/node_modules/.nobody-home`). -double_quoted='"[^"]*"' +double_quoted='"([^"\\]|\\.)*"' single_quoted="'[^']*'" unquoted="([^[:space:]'\"\\\\]|\\\\.)*" shell_word="${unquoted}((${double_quoted}|${single_quoted})${unquoted})*" @@ -116,7 +120,6 @@ variable_setting="[A-Za-z_][A-Za-z0-9_]*=${shell_word}[[:space:]]+" # command are shell builtins, so they have no path. # setpriv is not a wrapper here: a run through setpriv is the nobody run the # reply asks for, so the runner after it never counts. -optional_command_folder='([^[:space:]]*/)?' env_flags="([[:space:]]+((-[0iv]*[uC]|--unset|--chdir)[[:space:]]+${shell_word}|-[^[:space:]]*))*" timeout_wrapper="${optional_command_folder}timeout${flags_with_optional_argument}[[:space:]]+[0-9][^[:space:]]*" shell_flags='([[:space:]]+([-+][[:alpha:]]*[oO][[:alpha:]]*[[:space:]]+[^[:space:]]+|[-+][^[:space:]]*))*' @@ -178,7 +181,7 @@ exempt_suite_run="${vitest_exempt_suite_run}|${npm_exempt_suite_run}" # - cd_to_quoted_absolute_path matches a quoted path that starts with / and # holds no $, backtick or ~, so the shell would use it as written. # - cd_to_relative_path matches an unquoted relative path. Its first -# character cannot be - (`cd -` or a flag), # (a comment, which leaves a +# character cannot be - or \- (`cd -` or a flag), # (a comment, which leaves a # bare `cd`), or ~, $ or a quote, which only the shell can expand. # - any_cd matches any cd, pushd or popd. The walk tries it last, so it # catches the rest: a bare `cd`, with or without a comment after it, @@ -187,23 +190,8 @@ directory_change="^[[:space:]]*(cd|pushd)[[:space:]]+" escaped_path_character='([^[:space:]\\]|\\.)' cd_to_absolute_path="${directory_change}(/${escaped_path_character}*)" cd_to_quoted_absolute_path="${directory_change}[\"'](/[^\"'\$\`~]*)[\"']" -cd_to_relative_path="${directory_change}(([^-~#\"'\$[:space:]\\\\]|\\\\.)${escaped_path_character}*)" +cd_to_relative_path="${directory_change}(([^-~#\"'\$[:space:]\\\\]|\\\\[^-])${escaped_path_character}*)" leading_command_prefix="^[[:space:]]*${command_prefix}" - -# The shell drops the backslash in front of an escaped character in an -# unquoted path, so the walk does the same to a path it takes from a cd. -# Bash, not sed: the hook runs no tool it does not need. -backslash_escape='^([^\\]*)\\(.)(.*)$' -remove_backslash_escapes() { - local remaining_path="$1" - local unescaped_path="" - - while [[ "${remaining_path}" =~ ${backslash_escape} ]]; do - unescaped_path+="${BASH_REMATCH[1]}${BASH_REMATCH[2]}" - remaining_path="${BASH_REMATCH[3]}" - done - printf '%s' "${unescaped_path}${remaining_path}" -} any_cd="^[[:space:]]*${command_prefix}(cd|pushd|popd)([[:space:]]|$)" # The command is split into the commands it runs, one per line, and only the @@ -265,8 +253,11 @@ while IFS= read -r split_command; do # match's length. cd_candidate="${split_command:${#BASH_REMATCH[0]}}" fi + # read without -r drops each escaping backslash, as the shell does, and as + # a builtin it costs no fork on a command with thousands of cds. if [[ "${cd_candidate}" =~ ${cd_to_absolute_path} ]]; then - current_dir="$(remove_backslash_escapes "${BASH_REMATCH[2]}")" + # shellcheck disable=SC2162 + IFS= read current_dir <<<"${BASH_REMATCH[2]}" continue fi if [[ "${cd_candidate}" =~ ${cd_to_quoted_absolute_path} ]]; then @@ -274,7 +265,9 @@ while IFS= read -r split_command; do continue fi if [[ "${cd_candidate}" =~ ${cd_to_relative_path} ]]; then - current_dir="${current_dir}/$(remove_backslash_escapes "${BASH_REMATCH[2]}")" + # shellcheck disable=SC2162 + IFS= read relative_path <<<"${BASH_REMATCH[2]}" + current_dir="${current_dir}/${relative_path}" continue fi if [[ "${cd_candidate}" =~ ${any_cd} ]]; then @@ -415,7 +408,7 @@ if [[ "${node_major_differs}" == true ]]; then nvmrc_node_path="$(node -p process.execPath 2>/dev/null)" || nvmrc_node_path="" nvmrc_node_version="${hook_node_version}" else - for nvm_dir in "${NVM_DIR:-}" "${HOME}/.nvm"; do + for nvm_dir in "${NVM_DIR:-}" "${HOME:-}/.nvm"; do [[ -n "${nvm_dir}" && -d "${nvm_dir}/versions/node" ]] || continue # nvm keeps each node three folders below versions/node, in a folder # named by its version: /versions/node/v24.11.0/bin/node. diff --git a/src/__tests__/require-nonroot-tests.test.ts b/src/__tests__/require-nonroot-tests.test.ts index 2ba5808bc..64d4e616e 100644 --- a/src/__tests__/require-nonroot-tests.test.ts +++ b/src/__tests__/require-nonroot-tests.test.ts @@ -172,6 +172,8 @@ type HookRunOptions = { extraCaCerts?: string /** The hook's NVM_DIR. Unset by default, so the hook looks only in ~/.nvm. */ nvmDir?: string + /** Whether the hook gets a HOME. Defaults to true. */ + hasHome?: boolean /** The umask the hook starts with, as `umask` takes it. Defaults to the * runner's own. */ umask?: string @@ -301,7 +303,7 @@ const hookEnv = (options: Omit): Record // own environment never reaches the hook. return { PATH: options.path ?? `${fixture.stubBinDir}:${runnerPath()}`, - HOME: fixture.outsideDir, + ...(options.hasHome === false ? {} : { HOME: fixture.outsideDir }), STUB_SESSION_UID: String(options.sessionUid ?? 0), STUB_NOBODY_EXISTS: options.nobodyExists === false ? "0" : "1", STUB_NOBODY_UID: String(uid), @@ -539,6 +541,13 @@ describe("require-nonroot-tests hook", () => { label: "env with -C and a folder holding an escaped space", command: "env -C /my\\ dir npm test", }, + { + label: "env with -u and a name holding an escaped quote", + command: 'env -u "a\\"b" npm test', + }, + { label: "a double-quoted value holding an escaped quote", command: 'A="a\\"b" npm test' }, + { label: "npm by its path", command: "/usr/local/bin/npm test" }, + { label: "npx by its path", command: "/usr/local/bin/npx vitest run" }, { label: "a double-quoted value with a space", command: 'FOO="a b" npm test' }, { label: "a value that mixes unquoted and quoted text", @@ -986,6 +995,28 @@ describe("require-nonroot-tests hook", () => { }) }) + it("copies the node in $NVM_DIR when HOME is unset", () => { + const fixture = createHookFixture() + const nvmDir = join(fixture.outsideDir, "nvm-dir") + writeNvmNodes(nvmDir, ["v24.1.0"]) + const nobodyNodeDir = nodeCopyFolder(fixture.checkout, "v24.1.0") + + const run = runHook({ + fixture, + stdin: payloadFor({ command: "npm test", cwd: fixture.checkout }), + nobodyNodeVersion: OTHER_NODE_VERSION, + sessionNodeVersion: OTHER_NODE_VERSION, + nvmDir, + hasHome: false, + }) + + expect(run).toEqual({ + status: 2, + stdout: "", + stderr: `${expectedRefusal(fixture.checkout, { nobodyNodeDir })}${expectedNodeCopyLine({ nobodyNodeVersion: OTHER_NODE_VERSION, copiedNodeVersion: "v24.1.0" })}`, + }) + }) + it("copies the node in ~/.nvm when $NVM_DIR has none of the .nvmrc major version", () => { const fixture = createHookFixture() const nvmDir = join(fixture.outsideDir, "nvm-dir") @@ -1491,6 +1522,8 @@ describe("require-nonroot-tests hook", () => { { label: "a double-quoted path", target: '"src"' }, { label: "a single-quoted path", target: "'src'" }, { label: "the previous folder", target: "-" }, + // The shell drops the backslash, so cd reads -src as flags. + { label: "an escaped leading -", target: "\\-src" }, ])( "prepares the session's checkout for a cd to $label, which only the shell can resolve", ({ target }) => { @@ -1561,6 +1594,21 @@ describe("require-nonroot-tests hook", () => { }) }) + it("prepares the session's checkout, not env's -C folder", () => { + const fixture = createHookFixture() + const otherCheckout = createOtherCheckout(fixture) + + const run = runCommandThroughHook(fixture, `env -C ${otherCheckout} npm test`) + // A cd to the other checkout moves the run there, so the fixture can + // tell the two checkouts apart. + const controlRun = runCommandThroughHook(fixture, `cd ${otherCheckout} && npm test`) + + expect({ run, controlRun }).toEqual({ + run: { status: 2, stdout: "", stderr: expectedRefusal(fixture.checkout) }, + controlRun: { status: 2, stdout: "", stderr: expectedRefusal(otherCheckout) }, + }) + }) + it("stops a test run thousands of commands after the cd that names its checkout", () => { const fixture = createHookFixture() const otherCommands = Array.from({ length: 5000 }, (_, lineIndex) => `echo line ${lineIndex}`) From 5aa39a4c274412ffce58db7ef38b57692fe9cf8d Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 23:11:56 +0000 Subject: [PATCH 17/19] test(hooks): name the folder an escaped leading - would reach The case created a folder named with the backslash, so a hook that followed the cd fell back to the session's checkout and passed anyway. Co-Authored-By: Claude Opus 5.5 --- src/__tests__/require-nonroot-tests.test.ts | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/__tests__/require-nonroot-tests.test.ts b/src/__tests__/require-nonroot-tests.test.ts index 64d4e616e..3c26f8763 100644 --- a/src/__tests__/require-nonroot-tests.test.ts +++ b/src/__tests__/require-nonroot-tests.test.ts @@ -1522,8 +1522,6 @@ describe("require-nonroot-tests hook", () => { { label: "a double-quoted path", target: '"src"' }, { label: "a single-quoted path", target: "'src'" }, { label: "the previous folder", target: "-" }, - // The shell drops the backslash, so cd reads -src as flags. - { label: "an escaped leading -", target: "\\-src" }, ])( "prepares the session's checkout for a cd to $label, which only the shell can resolve", ({ target }) => { @@ -1549,6 +1547,22 @@ describe("require-nonroot-tests hook", () => { }, ) + it("prepares the session's checkout for a cd to an escaped leading -, which cd reads as a flag", () => { + const fixture = createHookFixture() + const otherCheckout = createOtherCheckout(fixture) + // The folder the path names once the shell drops the backslash: a hook + // that followed the cd would land in the other checkout. + mkdirSync(join(otherCheckout, "-src")) + + const run = runCommandThroughHook(fixture, `cd ${otherCheckout} && cd \\-src && npm test`) + const controlRun = runCommandThroughHook(fixture, `cd ${otherCheckout} && npm test`) + + expect({ run, controlRun }).toEqual({ + run: { status: 2, stdout: "", stderr: expectedRefusal(fixture.checkout) }, + controlRun: { status: 2, stdout: "", stderr: expectedRefusal(otherCheckout) }, + }) + }) + it("prepares the session's checkout for a cd followed only by a comment, which bash runs as a bare cd", () => { const fixture = createHookFixture() const otherCheckout = createOtherCheckout(fixture) From e9411af1e9339d7b945d799a632a3a8285bd5331 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 23:43:38 +0000 Subject: [PATCH 18/19] chore(hooks): tell a setting's value from a command path, and follow \/ and += forms - A folder holding = is a setting's value, so `FOO=/x/npm test`, which runs the test builtin, is no longer stopped as npm named by path. - node named by path running vitest.mjs is a runner, as npm, npx and vitest by path already are. - A VAR+=value setting before the command is read as one. - A cd path starting with \/ is absolute, not relative. - A .nvmrc that starts with a space still names its version. Co-Authored-By: Claude Opus 5.5 --- .claude/hooks/require-nonroot-tests.sh | 42 +++++++++++--------- src/__tests__/require-nonroot-tests.test.ts | 43 +++++++++++++++++++++ 2 files changed, 66 insertions(+), 19 deletions(-) diff --git a/.claude/hooks/require-nonroot-tests.sh b/.claude/hooks/require-nonroot-tests.sh index dcf73e4c0..e5414f600 100755 --- a/.claude/hooks/require-nonroot-tests.sh +++ b/.claude/hooks/require-nonroot-tests.sh @@ -73,19 +73,20 @@ flags_with_optional_argument='([[:space:]]+-[^[:space:]]*([[:space:]]+[^-[:space # out test:remote-boot and test:cli-pty, the two suites exempt from this # hook because they cannot run as nobody; the exemptions below give each # reason. -# npm and npx may be named by path (`/usr/local/bin/npm test`), as vitest and -# the wrappers below may. -optional_command_folder='([^[:space:]]*/)?' -vitest_binary='([^[:space:]]*/)?vitest(@[^[:space:]]*)?' -vitest_runner="(${optional_command_folder}npx${flags_with_optional_argument}[[:space:]]+)?${vitest_binary}|node[[:space:]]+[^[:space:]]*vitest\.mjs" +# vitest, npm, npx and node may be named by path (`/usr/local/bin/npm test`), +# as the wrappers below may. A folder holding = is a setting's value, not a +# command's path, so `FOO=/x/npm test` runs test, not npm. +optional_command_folder='([^[:space:]=]*/)?' +vitest_binary="${optional_command_folder}vitest(@[^[:space:]]*)?" +vitest_runner="(${optional_command_folder}npx${flags_with_optional_argument}[[:space:]]+)?${vitest_binary}|${optional_command_folder}node[[:space:]]+[^[:space:]]*vitest\.mjs" npm_exec_runner="${optional_command_folder}npm${flags_with_optional_argument}[[:space:]]+(exec|x)${flags_with_optional_argument}[[:space:]]+${vitest_binary}" npm_runner="${optional_command_folder}npm${flags_with_optional_argument}[[:space:]]+(test|t|run(-script)?${flags_with_optional_argument}[[:space:]]+(test|test:coverage|test:watch|snapshot:update))" test_runner="${vitest_runner}|${npm_exec_runner}|${npm_runner}" -# variable_setting matches a VAR=value setting before the command -# (`CI=1 npm test`). Its value is a shell_word: unquoted text with any number -# of quoted parts, each followed by more unquoted text, as in the reply's own -# prefix (`PATH=/copy:"$PATH"`). +# variable_setting matches a VAR=value or VAR+=value setting before the +# command (`CI=1 npm test`). Its value is a shell_word: unquoted text with +# any number of quoted parts, each followed by more unquoted text, as in the +# reply's own prefix (`PATH=/copy:"$PATH"`). # - double_quoted matches text in double quotes, where a backslash escapes # the next character (`"a\"b"`). # - single_quoted matches text in single quotes, with no ' inside. @@ -96,7 +97,7 @@ double_quoted='"([^"\\]|\\.)*"' single_quoted="'[^']*'" unquoted="([^[:space:]'\"\\\\]|\\\\.)*" shell_word="${unquoted}((${double_quoted}|${single_quoted})${unquoted})*" -variable_setting="[A-Za-z_][A-Za-z0-9_]*=${shell_word}[[:space:]]+" +variable_setting="[A-Za-z_][A-Za-z0-9_]*\\+?=${shell_word}[[:space:]]+" # wrapper matches these commands, which run the command after them: # - env and time, with flags that take no argument, because a flag's argument @@ -175,22 +176,23 @@ exempt_suite_run="${vitest_exempt_suite_run}|${npm_exempt_suite_run}" # test'`) is cut off first, with leading_command_prefix, so the first three # patterns start at cd or pushd and the path is the second group of their # match. any_cd accepts that prefix itself for the grep below. -# - cd_to_absolute_path matches an unquoted path that starts with /. In it, -# and in cd_to_relative_path, a backslash escapes the next character -# (`cd /my\ repo`), and the walk drops the backslash. +# - cd_to_absolute_path matches an unquoted path that starts with / or \/. +# In it, and in cd_to_relative_path, a backslash escapes the next +# character (`cd /my\ repo`), and the walk drops the backslash. # - cd_to_quoted_absolute_path matches a quoted path that starts with / and # holds no $, backtick or ~, so the shell would use it as written. # - cd_to_relative_path matches an unquoted relative path. Its first -# character cannot be - or \- (`cd -` or a flag), # (a comment, which leaves a -# bare `cd`), or ~, $ or a quote, which only the shell can expand. +# character cannot be - or \- (`cd -` or a flag), \/ (an absolute path), +# # (a comment, which leaves a bare `cd`), or ~, $ or a quote, which only +# the shell can expand. # - any_cd matches any cd, pushd or popd. The walk tries it last, so it # catches the rest: a bare `cd`, with or without a comment after it, # `cd -`, popd, and a ~, $ or quoted path the others do not take. directory_change="^[[:space:]]*(cd|pushd)[[:space:]]+" escaped_path_character='([^[:space:]\\]|\\.)' -cd_to_absolute_path="${directory_change}(/${escaped_path_character}*)" +cd_to_absolute_path="${directory_change}(\\\\?/${escaped_path_character}*)" cd_to_quoted_absolute_path="${directory_change}[\"'](/[^\"'\$\`~]*)[\"']" -cd_to_relative_path="${directory_change}(([^-~#\"'\$[:space:]\\\\]|\\\\[^-])${escaped_path_character}*)" +cd_to_relative_path="${directory_change}(([^-~#\"'\$[:space:]\\\\]|\\\\[^-/])${escaped_path_character}*)" leading_command_prefix="^[[:space:]]*${command_prefix}" any_cd="^[[:space:]]*${command_prefix}(cd|pushd|popd)([[:space:]]|$)" @@ -361,8 +363,10 @@ chmod -R go-w "${nobody_writable[@]}" # node_major_differs is true when .nvmrc names a number (24 or v24.1.0), not # an alias such as lts/*, and nobody's node has another major version. nvmrc_version="$(cat "${checkout}/.nvmrc" 2>/dev/null)" || nvmrc_version="" -# Cut the text at its first space, tab or line break, so a CRLF line end or a -# comment after the version does not hide it. +# Drop any leading spaces, tabs or line breaks, then cut the text at the +# next one, so a CRLF line end or a comment after the version does not hide +# it. +nvmrc_version="${nvmrc_version#"${nvmrc_version%%[![:space:]]*}"}" nvmrc_version="${nvmrc_version%%[[:space:]]*}" nvmrc_major="${nvmrc_version#v}" nvmrc_major="${nvmrc_major%%.*}" diff --git a/src/__tests__/require-nonroot-tests.test.ts b/src/__tests__/require-nonroot-tests.test.ts index 3c26f8763..b1dbbc483 100644 --- a/src/__tests__/require-nonroot-tests.test.ts +++ b/src/__tests__/require-nonroot-tests.test.ts @@ -548,6 +548,11 @@ describe("require-nonroot-tests hook", () => { { label: "a double-quoted value holding an escaped quote", command: 'A="a\\"b" npm test' }, { label: "npm by its path", command: "/usr/local/bin/npm test" }, { label: "npx by its path", command: "/usr/local/bin/npx vitest run" }, + { + label: "node by its path running vitest.mjs", + command: "/usr/local/bin/node node_modules/vitest/vitest.mjs run", + }, + { label: "a += setting", command: "CI+=1 npm test" }, { label: "a double-quoted value with a space", command: 'FOO="a b" npm test' }, { label: "a value that mixes unquoted and quoted text", @@ -620,6 +625,9 @@ describe("require-nonroot-tests hook", () => { // FOO is the command env runs. { label: "env with -u unsetting u, then FOO", command: "env -uu FOO npm test" }, { label: "env with -u unsetting C, then FOO", command: "env -uC FOO npm test" }, + // A value ending in /npm is not npm by path: the shell runs test. + { label: "a setting whose value ends in /npm", command: "FOO=/x/npm test" }, + { label: "a setting whose value ends in /npx", command: "FOO=bin/npx -y vitest run" }, { label: "a quoted grep pattern that a | splits after a runner name", command: "grep -E 'npm test|vitest' src/", @@ -1274,6 +1282,24 @@ describe("require-nonroot-tests hook", () => { }) }) + it("compares the major versions when .nvmrc starts with a space", () => { + const fixture = createHookFixture() + writeFileSync(join(fixture.checkout, ".nvmrc"), ` ${NVMRC_VERSION}\n`) + + const run = runHook({ + fixture, + stdin: payloadFor({ command: "npm test", cwd: fixture.checkout }), + nobodyNodeVersion: "v22.22.0", + sessionNodeVersion: "v20.19.0", + }) + + expect(run).toEqual({ + status: 2, + stdout: "", + stderr: `${expectedRefusal(fixture.checkout)}nobody's node is v22.22.0, not the 24 that .nvmrc names and CI runs, so a result can differ from CI's.\n`, + }) + }) + it("leaves the version line out when .nvmrc names an alias rather than a version", () => { const fixture = createHookFixture() writeFileSync(join(fixture.checkout, ".nvmrc"), "lts/*\n") @@ -1547,6 +1573,23 @@ describe("require-nonroot-tests hook", () => { }, ) + it("prepares the checkout a cd names with an escaped leading /", () => { + const fixture = createHookFixture() + const otherCheckout = createOtherCheckout(fixture) + + // The session's folder is in no checkout, so only the cd's path can + // reach one. + const run = runHook({ + fixture, + stdin: payloadFor({ + command: `cd \\${otherCheckout} && npm test`, + cwd: fixture.outsideDir, + }), + }) + + expect(run).toEqual({ status: 2, stdout: "", stderr: expectedRefusal(otherCheckout) }) + }) + it("prepares the session's checkout for a cd to an escaped leading -, which cd reads as a flag", () => { const fixture = createHookFixture() const otherCheckout = createOtherCheckout(fixture) From 5707820f8fbcf2c2f1381c129362bba392952226 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 00:35:14 +0000 Subject: [PATCH 19/19] chore(hooks): stop reading quoted text and heredoc bodies as test runs The split breaks a command at ;, &, |, ( and backticks even inside quotes, so a commit message, an inline script or a heredoc that only mentioned a test run produced a line starting with one, and the hook blocked a command that runs no tests. The node step that parses the payload now blanks those characters inside quotes and drops heredoc bodies and comments, unless a word of the command names bash, sh or zsh, whose quoted text and heredocs run. A $( ) inside double quotes is still read as commands. Co-Authored-By: Claude Opus 5.5 --- .claude/hooks/require-nonroot-tests.sh | 176 +++++++++++++++++++- src/__tests__/require-nonroot-tests.test.ts | 22 +++ 2 files changed, 190 insertions(+), 8 deletions(-) diff --git a/.claude/hooks/require-nonroot-tests.sh b/.claude/hooks/require-nonroot-tests.sh index e5414f600..09cadd5e6 100755 --- a/.claude/hooks/require-nonroot-tests.sh +++ b/.claude/hooks/require-nonroot-tests.sh @@ -35,17 +35,177 @@ fi # - Without node, or with a payload node cannot parse, node prints nothing. # Each read then fails and leaves its variable empty (`|| true` keeps set -e # from ending the hook), and an empty command lets the call through. -# - The script is single-quoted on purpose: its ${...} is JavaScript, not shell. +# - The script is single-quoted on purpose: its ${...} is JavaScript, not +# shell, and it writes a single quote as \u0027. +# - The command it prints has the text the command only passes as data +# blanked out, so the split below never reads a mention of a test run in +# a commit message, a script or a heredoc as a run: +# - Inside quotes, each ;, &, |, (, backtick and line break becomes a +# space (`git commit -m "... (npm test) ..."`, `node -e '/(npx|npm)/'`). +# - A heredoc's body is dropped (`python3 - <<'EOF'`), and so is a comment. +# - Both stay as written when a word of the command names bash, sh or zsh, +# because the shell runs that text (`bash -c 'cd /repo && npm test'`, +# `bash <<'EOF'`). A $( ) inside double quotes runs too, so its text is +# read as commands (`echo "$(npm test)"`). # shellcheck disable=SC2016 { IFS= read -r -d '' tool_command || true IFS= read -r -d '' payload_cwd || true } < <(node -e ' + const QUOTE = "\u0027" + const SEPARATORS = ";&|(`\n" + const SHELL_NAME = /(^|\/)(bash|sh|zsh)$/ + const HEREDOC_OPERATOR = /^<<(-?)[ \t]*(?:"([^"]*)"|\u0027([^\u0027]*)\u0027|([^\s;&|()<>]+))/ + + const blankSeparators = (text) => text.replace(/[;&|(`\n]/g, " ") + + const maskDataText = (command) => { + // A parser threading state character by character: output, position, + // the context stack, and the current word are all sequential state. + let output = "" + let index = 0 + let word = "" + let commandRunsShell = false + const pendingHeredocs = [] + // The bottom context is the command itself; "double" is double-quoted + // text; "substitution" is a $( ) inside double quotes. + const contexts = [{ kind: "command" }] + + const endWord = () => { + if (SHELL_NAME.test(word)) commandRunsShell = true + word = "" + } + const startCommand = () => { + word = "" + commandRunsShell = false + } + const readHeredocBodies = () => { + for (const heredoc of pendingHeredocs) { + while (index < command.length) { + const newlineIndex = command.indexOf("\n", index) + const lineEnd = newlineIndex === -1 ? command.length : newlineIndex + const line = command.slice(index, lineEnd) + const delimiterLine = heredoc.stripsTabs ? line.replace(/^\t+/, "") : line + index = lineEnd + 1 + if (delimiterLine === heredoc.delimiter) break + output += `${heredoc.bodyRuns ? line : ""}\n` + } + } + pendingHeredocs.length = 0 + } + + while (index < command.length) { + const character = command[index] + const next = command[index + 1] ?? "" + const context = contexts[contexts.length - 1] + + if (context.kind === "double") { + if (character === "\\") { + output += character + (commandRunsShell ? next : blankSeparators(next)) + index += 2 + continue + } + if (character === "\"") { + contexts.pop() + output += character + index += 1 + continue + } + if (character === "$" && next === "(") { + contexts.push({ kind: "substitution", depth: 0, word, commandRunsShell }) + startCommand() + output += "$(" + index += 2 + continue + } + output += commandRunsShell ? character : blankSeparators(character) + index += 1 + continue + } + + if (character === "\\") { + // A line continuation joins two lines into one command. + output += next === "\n" ? " " : character + blankSeparators(next) + word += next + index += 2 + continue + } + if (character === QUOTE) { + const closing = command.indexOf(QUOTE, index + 1) + const quoteEnd = closing === -1 ? command.length : closing + const quotedText = command.slice(index + 1, quoteEnd) + output += QUOTE + (commandRunsShell ? quotedText : blankSeparators(quotedText)) + output += closing === -1 ? "" : QUOTE + word += quotedText + index = quoteEnd + 1 + continue + } + if (character === "\"") { + contexts.push({ kind: "double" }) + output += character + index += 1 + continue + } + if (character === "#" && word === "") { + const lineEnd = command.indexOf("\n", index) + index = lineEnd === -1 ? command.length : lineEnd + continue + } + const heredocOperator = character === "<" && next === "<" ? HEREDOC_OPERATOR.exec(command.slice(index)) : null + if (heredocOperator) { + const [operatorText, tabsFlag, doubleQuoted, singleQuoted, bare] = heredocOperator + pendingHeredocs.push({ + delimiter: doubleQuoted ?? singleQuoted ?? bare, + stripsTabs: tabsFlag === "-", + bodyRuns: commandRunsShell || SHELL_NAME.test(word), + }) + output += operatorText + index += operatorText.length + continue + } + if (character === "\n") { + output += character + index += 1 + readHeredocBodies() + startCommand() + continue + } + if (context.kind === "substitution" && character === ")") { + if (context.depth === 0) { + contexts.pop() + word = context.word + commandRunsShell = context.commandRunsShell + output += character + index += 1 + continue + } + context.depth -= 1 + } + if (SEPARATORS.includes(character)) { + if (context.kind === "substitution" && character === "(") context.depth += 1 + output += character + index += 1 + startCommand() + continue + } + if (/\s/.test(character)) { + endWord() + output += character + index += 1 + continue + } + word += character + output += character + index += 1 + } + return output + } + const chunks = [] // Buffer.concat runs before toString, because a multi-byte character can span two chunks. process.stdin.on("data", (chunk) => chunks.push(chunk)).on("end", () => { const parsed = JSON.parse(Buffer.concat(chunks).toString("utf8")) - process.stdout.write(`${parsed.tool_input?.command ?? ""}\0${parsed.cwd ?? ""}\0`) + process.stdout.write(`${maskDataText(parsed.tool_input?.command ?? "")}\0${parsed.cwd ?? ""}\0`) }) ' 2>/dev/null) @@ -112,10 +272,10 @@ variable_setting="[A-Za-z_][A-Za-z0-9_]*\\+?=${shell_word}[[:space:]]+" # - bash, sh and zsh with their flags, where a group of one-letter flags that # holds o or O takes an argument (`bash -euo pipefail -c`, # `bash -O extglob -c`), then the opening quote of the command they run and -# any spaces after it (`bash -lc 'npm test'`). The split below ignores -# quotes, so a run or a cd after ; or && inside the quoted text counts too -# (`bash -c 'npm ci && cd /repo && npm test'`), and a mention there -# (`bash -c 'echo npm test'`) does not. +# any spaces after it (`bash -lc 'npm test'`). The quoted text a shell runs +# reaches the split as written, so a run or a cd after ; or && inside it +# counts too (`bash -c 'npm ci && cd /repo && npm test'`), and a mention +# there (`bash -c 'echo npm test'`) does not. # - Each of env, time, timeout and the shells may be named by path # (`/usr/bin/env npm test`), as the reply's own prefix names env. exec and # command are shell builtins, so they have no path. @@ -138,8 +298,8 @@ command_prefix="(${wrapper}|${shell_keyword}|${variable_setting})*" # followed only by spaces and then a redirection (`bash -c 'npm test' 2>`), a # ) (`$(bash -c 'npm test')`), a comment or the end. So `npm run testx` and # `npm run test:cli-pty` are not runs, and neither is the `vitest' src/` that -# `grep -E 'npm test|vitest' src/` splits into. Inside the double quotes, -# `\\)` reaches the regex as `\)`, a literal ). +# a shell's quoted `grep -E 'npm test|vitest' src/` splits into. Inside the +# double quotes, `\\)` reaches the regex as `\)`, a literal ). runner_end="([[:space:]]|\\)|[\"'][[:space:]]*([0-9]*[<>]|\\)|$)|[\"'][[:space:]]+#|$)" # A command that runs tests starts with the runner, after any prefix. A diff --git a/src/__tests__/require-nonroot-tests.test.ts b/src/__tests__/require-nonroot-tests.test.ts index b1dbbc483..f53d2ba9e 100644 --- a/src/__tests__/require-nonroot-tests.test.ts +++ b/src/__tests__/require-nonroot-tests.test.ts @@ -505,6 +505,10 @@ describe("require-nonroot-tests hook", () => { { label: "node running vitest.mjs", command: "node node_modules/vitest/vitest.mjs run" }, { label: "npm run snapshot:update", command: "npm run snapshot:update" }, { label: "a subshell", command: "(cd sub; npm test)" }, + { label: "a line after a comment holding an apostrophe", command: "# it's a run\nnpm test" }, + { label: "a bash heredoc", command: "bash <<'EOF'\nnpm test\nEOF" }, + { label: "a line after a cat heredoc", command: "cat <notes\nhello\nEOF\nnpm test" }, + { label: "a command substitution inside double quotes", command: 'echo "$(npm test)"' }, { label: "a subshell inside command substitution", command: "out=$( (npm test) )" }, { label: "a backtick substitution", command: "out=`npm test 2>&1`" }, { label: "a variable setting before npx", command: "CI=1 npx vitest run" }, @@ -625,6 +629,24 @@ describe("require-nonroot-tests hook", () => { // FOO is the command env runs. { label: "env with -u unsetting u, then FOO", command: "env -uu FOO npm test" }, { label: "env with -u unsetting C, then FOO", command: "env -uC FOO npm test" }, + // Quoted text and heredoc bodies a command only passes as data. + { + label: "a commit message that mentions a run in parentheses", + command: + 'git commit -m "fix: x\n\n- the prefix (/usr/bin/env HOME=/h npm test) went\n through"', + }, + { + label: "a commit message from a cat heredoc that mentions a run", + command: `git commit -m "$(cat <<'EOF'\nfix: the (npm test) run "worked"\nEOF\n)"`, + }, + { + label: "a python heredoc whose body mentions a run", + command: "python3 - <<'EOF'\nprint(\"(`env -iu NAME npm test`)\")\nEOF", + }, + { + label: "a node -e script whose regex names a run", + command: "node -e 'if (!/^(npx|npm|npx vitest run src)/.test(text)) report()'", + }, // A value ending in /npm is not npm by path: the shell runs test. { label: "a setting whose value ends in /npm", command: "FOO=/x/npm test" }, { label: "a setting whose value ends in /npx", command: "FOO=bin/npx -y vitest run" },