From 56b7333a03a63629cc7ea44f5fe81751ea800825 Mon Sep 17 00:00:00 2001 From: Gabriel Gordon-Hall Date: Tue, 1 Sep 2026 18:46:25 +0100 Subject: [PATCH 1/3] fix(postinstall): do not install skills as another package's dependency MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `npm_config_global` is set for the whole of a global install, including the lifecycle scripts of its dependencies. So a package that merely depends on Context Tree triggered the auto-install: `npm i -g some-tool` wrote six skill directories into the user's own ~/.claude and ~/.codex as an invisible side effect of installing an unrelated tool. Also require that this package is the install target rather than a nested copy. A directly installed package sits in npm's prefix, whose parent is not a package; a dependency sits inside the owning package's node_modules. `scripts/package-e2e.mjs` simulated a global install by forcing npm_config_global on the locally installed copy, which is precisely the layout this change now refuses. That shortcut was sound when the flag was the only signal, but it no longer models a global install, so the e2e installs for real into a temporary prefix instead — the layout npm produces is the whole basis of the distinction. The nested case it used to stand in for is now asserted explicitly, in the direction we actually guarantee. Verified with `npm run check:package`, and separately against a packed tarball installed into an isolated prefix: a direct global install still installs the skills, a global install of a dependent package prints the hint and writes nothing, and removing the guard reproduces the original behaviour from the same nested layout. Co-Authored-By: Claude Opus 5 (1M context) --- scripts/package-e2e.mjs | 28 +++++++++++++++++++++-- scripts/postinstall.mjs | 29 +++++++++++++++++++----- tests/install.test.ts | 49 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 98 insertions(+), 8 deletions(-) diff --git a/scripts/package-e2e.mjs b/scripts/package-e2e.mjs index ad64dc2..9dc0a57 100644 --- a/scripts/package-e2e.mjs +++ b/scripts/package-e2e.mjs @@ -136,12 +136,36 @@ try { "a local install must not write skills to the home directory", ); - // A global install does, which is the documented path. - const globalPostinstall = spawnSync(process.execPath, [join(installedPackage, "scripts/postinstall.mjs")], { + // Neither does a global install of some *other* package that depends on this one: npm sets + // npm_config_global for its dependencies too, so the flag alone cannot authorize the write. + // This copy is nested inside the consumer, which is exactly that layout. + const nestedPostinstall = spawnSync(process.execPath, [join(installedPackage, "scripts/postinstall.mjs")], { cwd: consumerRoot, encoding: "utf8", env: { ...npmEnvironment, npm_config_global: "true" }, }); + assert.equal(nestedPostinstall.status, 0, "postinstall must never fail an install"); + assert.match(nestedPostinstall.stdout, /run `context-tree install`/u); + assert.equal( + existsSync(join(temporaryRoot, ".claude", "skills")), + false, + "a global install of a dependent package must not write skills to the home directory", + ); + + // A direct global install does, which is the documented path. Installing for real is what + // proves it, because the layout npm produces is the whole basis of the distinction. + const globalPrefix = join(temporaryRoot, "global-prefix"); + execFileSync("npm", ["install", "-g", "--prefix", globalPrefix, "--no-audit", "--no-fund", tarball], { + cwd: temporaryRoot, + env: npmEnvironment, + stdio: "pipe", + }); + const globallyInstalled = join(globalPrefix, "lib/node_modules/@first-tree-ai/context-tree"); + const globalPostinstall = spawnSync(process.execPath, [join(globallyInstalled, "scripts/postinstall.mjs")], { + cwd: temporaryRoot, + encoding: "utf8", + env: { ...npmEnvironment, npm_config_global: "true" }, + }); assert.equal(globalPostinstall.status, 0, "postinstall must never fail an install"); assert.match(globalPostinstall.stdout, /installed 6 skills for claude/u); for (const skill of SKILLS) { diff --git a/scripts/postinstall.mjs b/scripts/postinstall.mjs index f021f81..5fe6b7f 100644 --- a/scripts/postinstall.mjs +++ b/scripts/postinstall.mjs @@ -5,20 +5,37 @@ // A failure must never fail `npm install`: the CLI is still usable, and // `context-tree install` can be run by hand afterwards. // -// Only a global install writes to the home directory. Adding this package as a local -// dependency — including this repository's own `pnpm install` — must not silently -// modify the developer's agent configuration, so it just prints the command. +// Only a direct global install writes to the home directory. Being a dependency of +// something else — a local `pnpm install`, or a global install of a package that depends +// on this one — must not silently modify the developer's agent configuration, so it just +// prints the command. import { spawnSync } from "node:child_process"; -import { dirname, resolve } from "node:path"; +import { existsSync } from "node:fs"; +import { basename, dirname, join, resolve } from "node:path"; import { fileURLToPath } from "node:url"; -if (process.env.npm_config_global !== "true") { +const packageRoot = resolve(dirname(fileURLToPath(import.meta.url)), ".."); + +// A global install of some *other* package that depends on this one also sets +// npm_config_global, so the flag alone does not mean this package is the install target. +// Such a copy sits inside the owning package's node_modules, whereas a directly installed +// one sits in npm's own prefix, which is not a package. +function ownedByAnotherPackage() { + for (let directory = packageRoot; ; ) { + const parent = dirname(directory); + if (parent === directory) return false; + if (basename(directory) === "node_modules") return existsSync(join(parent, "package.json")); + directory = parent; + } +} + +if (process.env.npm_config_global !== "true" || ownedByAnotherPackage()) { process.stdout.write("Context Tree: run `context-tree install` to add the skills to your agent.\n"); process.exit(0); } -const cli = resolve(dirname(fileURLToPath(import.meta.url)), "..", "dist", "cli", "index.mjs"); +const cli = join(packageRoot, "dist", "cli", "index.mjs"); const result = spawnSync(process.execPath, [cli, "install"], { encoding: "utf8" }); if (result.error !== undefined || result.status !== 0) { diff --git a/tests/install.test.ts b/tests/install.test.ts index cf6bfd9..f4a431c 100644 --- a/tests/install.test.ts +++ b/tests/install.test.ts @@ -1,3 +1,4 @@ +import { spawnSync } from "node:child_process"; import { existsSync, lstatSync, @@ -105,3 +106,51 @@ describe("skill installation", () => { expect(() => installSkills({ hosts: ["claude"], projectPath: root })).toThrow(/real directory/u); }); }); + +/** + * The postinstall script is the only code that writes to a user's agent configuration without + * being asked, so its guard is worth testing directly. `npm_config_global` alone is not enough: + * npm sets it for every dependency of a global install too, so a package that merely depends on + * Context Tree would otherwise install skills as a side effect. + */ +describe("postinstall guard", () => { + const script = resolve(import.meta.dirname, "..", "scripts", "postinstall.mjs"); + + /** Run the guard from a copy of the package placed at `packageRoot`, with an isolated home. */ + function runFrom(packageRoot: string, home: string, global: boolean): string { + mkdirSync(join(packageRoot, "scripts"), { recursive: true }); + mkdirSync(join(packageRoot, "dist", "cli"), { recursive: true }); + writeFileSync(join(packageRoot, "scripts", "postinstall.mjs"), readFileSync(script, "utf8")); + // Stand in for the built CLI so a permitted run reports an install without doing one. + writeFileSync( + join(packageRoot, "dist", "cli", "index.mjs"), + 'process.stdout.write(JSON.stringify({ installed: [{ host: "codex", path: "p", skills: ["s"] }], skipped: [] }));\n', + ); + mkdirSync(join(home, ".codex"), { recursive: true }); + const result = spawnSync(process.execPath, [join(packageRoot, "scripts", "postinstall.mjs")], { + encoding: "utf8", + env: { HOME: home, PATH: process.env.PATH ?? "", ...(global ? { npm_config_global: "true" } : {}) }, + }); + return result.stdout; + } + + it("installs only for a direct global install, never as another package's dependency", () => { + const prefix = workspace(); + const home = workspace(); + + // A direct global install: the parent of `node_modules` is npm's prefix, not a package. + const direct = join(prefix, "lib", "node_modules", "@first-tree-ai", "context-tree"); + expect(runFrom(direct, home, true)).toContain("installed 1 skills"); + + // A dependency of a global install: the parent of `node_modules` is the owning package. + const nested = join(prefix, "lib", "node_modules", "open-tag", "node_modules", "@first-tree-ai", "context-tree"); + mkdirSync(join(prefix, "lib", "node_modules", "open-tag"), { recursive: true }); + writeFileSync(join(prefix, "lib", "node_modules", "open-tag", "package.json"), '{"name":"open-tag"}\n'); + expect(runFrom(nested, workspace(), true)).toContain("run `context-tree install`"); + }); + + it("stays inert for a local install even at the top level", () => { + const root = join(workspace(), "node_modules", "@first-tree-ai", "context-tree"); + expect(runFrom(root, workspace(), false)).toContain("run `context-tree install`"); + }); +}); From 566fe2e29f23faeb20614b4def9865f5bd6ce650 Mon Sep 17 00:00:00 2001 From: Gabriel Gordon-Hall Date: Wed, 2 Sep 2026 00:30:39 +0100 Subject: [PATCH 2/3] refactor(postinstall): rely on the package e2e for the guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The unit test added with the guard copied the script into fabricated install layouts and re-asserted the three cases `scripts/package-e2e.mjs` already covers against a real `npm install -g` of the packed tarball. The e2e run is the higher-fidelity check — the layout npm produces is the whole basis of the distinction — so keep only that one. Also fold the duplicated explanation into a single comment at the guard. --- scripts/package-e2e.mjs | 9 ++++---- scripts/postinstall.mjs | 27 +++++++++-------------- tests/install.test.ts | 49 ----------------------------------------- 3 files changed, 15 insertions(+), 70 deletions(-) diff --git a/scripts/package-e2e.mjs b/scripts/package-e2e.mjs index 9dc0a57..162c062 100644 --- a/scripts/package-e2e.mjs +++ b/scripts/package-e2e.mjs @@ -136,9 +136,8 @@ try { "a local install must not write skills to the home directory", ); - // Neither does a global install of some *other* package that depends on this one: npm sets - // npm_config_global for its dependencies too, so the flag alone cannot authorize the write. - // This copy is nested inside the consumer, which is exactly that layout. + // Nor does a global install of a package that depends on this one: npm sets npm_config_global + // for its dependencies too, and this copy is nested inside the consumer, which is that layout. const nestedPostinstall = spawnSync(process.execPath, [join(installedPackage, "scripts/postinstall.mjs")], { cwd: consumerRoot, encoding: "utf8", @@ -152,8 +151,8 @@ try { "a global install of a dependent package must not write skills to the home directory", ); - // A direct global install does, which is the documented path. Installing for real is what - // proves it, because the layout npm produces is the whole basis of the distinction. + // A direct global install does. Installing for real is what proves it, because the layout npm + // produces is the whole basis of the distinction. const globalPrefix = join(temporaryRoot, "global-prefix"); execFileSync("npm", ["install", "-g", "--prefix", globalPrefix, "--no-audit", "--no-fund", tarball], { cwd: temporaryRoot, diff --git a/scripts/postinstall.mjs b/scripts/postinstall.mjs index 5fe6b7f..74e7443 100644 --- a/scripts/postinstall.mjs +++ b/scripts/postinstall.mjs @@ -5,10 +5,8 @@ // A failure must never fail `npm install`: the CLI is still usable, and // `context-tree install` can be run by hand afterwards. // -// Only a direct global install writes to the home directory. Being a dependency of -// something else — a local `pnpm install`, or a global install of a package that depends -// on this one — must not silently modify the developer's agent configuration, so it just -// prints the command. +// Only a direct global install writes to the home directory; anything else just prints the +// command, so installing this package as a dependency never touches a developer's own agents. import { spawnSync } from "node:child_process"; import { existsSync } from "node:fs"; @@ -17,20 +15,17 @@ import { fileURLToPath } from "node:url"; const packageRoot = resolve(dirname(fileURLToPath(import.meta.url)), ".."); -// A global install of some *other* package that depends on this one also sets -// npm_config_global, so the flag alone does not mean this package is the install target. -// Such a copy sits inside the owning package's node_modules, whereas a directly installed -// one sits in npm's own prefix, which is not a package. -function ownedByAnotherPackage() { - for (let directory = packageRoot; ; ) { - const parent = dirname(directory); - if (parent === directory) return false; - if (basename(directory) === "node_modules") return existsSync(join(parent, "package.json")); - directory = parent; - } +// npm sets npm_config_global for a global install's dependencies too, so the flag alone does not +// identify the install target. A dependency copy sits inside the owning package's node_modules; a +// directly installed one sits in npm's own prefix, which is not a package. +function ownedByAnotherPackage(directory) { + const parent = dirname(directory); + if (parent === directory) return false; + if (basename(directory) === "node_modules") return existsSync(join(parent, "package.json")); + return ownedByAnotherPackage(parent); } -if (process.env.npm_config_global !== "true" || ownedByAnotherPackage()) { +if (process.env.npm_config_global !== "true" || ownedByAnotherPackage(packageRoot)) { process.stdout.write("Context Tree: run `context-tree install` to add the skills to your agent.\n"); process.exit(0); } diff --git a/tests/install.test.ts b/tests/install.test.ts index f4a431c..cf6bfd9 100644 --- a/tests/install.test.ts +++ b/tests/install.test.ts @@ -1,4 +1,3 @@ -import { spawnSync } from "node:child_process"; import { existsSync, lstatSync, @@ -106,51 +105,3 @@ describe("skill installation", () => { expect(() => installSkills({ hosts: ["claude"], projectPath: root })).toThrow(/real directory/u); }); }); - -/** - * The postinstall script is the only code that writes to a user's agent configuration without - * being asked, so its guard is worth testing directly. `npm_config_global` alone is not enough: - * npm sets it for every dependency of a global install too, so a package that merely depends on - * Context Tree would otherwise install skills as a side effect. - */ -describe("postinstall guard", () => { - const script = resolve(import.meta.dirname, "..", "scripts", "postinstall.mjs"); - - /** Run the guard from a copy of the package placed at `packageRoot`, with an isolated home. */ - function runFrom(packageRoot: string, home: string, global: boolean): string { - mkdirSync(join(packageRoot, "scripts"), { recursive: true }); - mkdirSync(join(packageRoot, "dist", "cli"), { recursive: true }); - writeFileSync(join(packageRoot, "scripts", "postinstall.mjs"), readFileSync(script, "utf8")); - // Stand in for the built CLI so a permitted run reports an install without doing one. - writeFileSync( - join(packageRoot, "dist", "cli", "index.mjs"), - 'process.stdout.write(JSON.stringify({ installed: [{ host: "codex", path: "p", skills: ["s"] }], skipped: [] }));\n', - ); - mkdirSync(join(home, ".codex"), { recursive: true }); - const result = spawnSync(process.execPath, [join(packageRoot, "scripts", "postinstall.mjs")], { - encoding: "utf8", - env: { HOME: home, PATH: process.env.PATH ?? "", ...(global ? { npm_config_global: "true" } : {}) }, - }); - return result.stdout; - } - - it("installs only for a direct global install, never as another package's dependency", () => { - const prefix = workspace(); - const home = workspace(); - - // A direct global install: the parent of `node_modules` is npm's prefix, not a package. - const direct = join(prefix, "lib", "node_modules", "@first-tree-ai", "context-tree"); - expect(runFrom(direct, home, true)).toContain("installed 1 skills"); - - // A dependency of a global install: the parent of `node_modules` is the owning package. - const nested = join(prefix, "lib", "node_modules", "open-tag", "node_modules", "@first-tree-ai", "context-tree"); - mkdirSync(join(prefix, "lib", "node_modules", "open-tag"), { recursive: true }); - writeFileSync(join(prefix, "lib", "node_modules", "open-tag", "package.json"), '{"name":"open-tag"}\n'); - expect(runFrom(nested, workspace(), true)).toContain("run `context-tree install`"); - }); - - it("stays inert for a local install even at the top level", () => { - const root = join(workspace(), "node_modules", "@first-tree-ai", "context-tree"); - expect(runFrom(root, workspace(), false)).toContain("run `context-tree install`"); - }); -}); From 6cfb2fdc02e1959fa7ef9dd0796443f6112b426e Mon Sep 17 00:00:00 2001 From: Gabriel Gordon-Hall Date: Wed, 2 Sep 2026 09:02:20 +0100 Subject: [PATCH 3/3] bump --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 795cf6b..367bf51 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@first-tree-ai/context-tree", - "version": "0.1.7", + "version": "0.1.8", "description": "Durable, structured project context for coding agents: a CLI plus framework-neutral skills.", "type": "module", "license": "Apache-2.0",