From cdc0a6ec30702f345d078613ea78cbe67d1d4af3 Mon Sep 17 00:00:00 2001 From: Aschenbath Date: Tue, 20 Oct 2026 16:15:27 +0800 Subject: [PATCH] feat: resolve package manager bins from package manifests Signed-off-by: Aschenbath --- README.md | 7 ++ sources/binUtils.ts | 64 +++++++++++++++++ sources/corepackUtils.ts | 21 +++--- tests/PackageBin.test.ts | 55 +++++++++++++++ tests/_registryServer.mjs | 10 ++- tests/binUtils.test.ts | 140 ++++++++++++++++++++++++++++++++++++++ 6 files changed, 283 insertions(+), 14 deletions(-) create mode 100644 sources/binUtils.ts create mode 100644 tests/PackageBin.test.ts create mode 100644 tests/binUtils.test.ts diff --git a/README.md b/README.md index e2adeb949..8a6e7ffcd 100644 --- a/README.md +++ b/README.md @@ -107,6 +107,13 @@ CommonJS module) or a `.tgz` file (which will be interpreted as a package, and the `"bin"` field of the `package.json` will be used to determine which file to use in the archive). +For tarball installations of npm, pnpm, and Yarn, Corepack also reads executable +paths from the downloaded package's `package.json`, so changes to the package's +binary layout do not require an update to Corepack. Only command names supported +by Corepack are exposed. Entry points must resolve to files inside the package; +missing optional aliases are omitted. Single-file Yarn distributions continue to +use Corepack's configured command names. + ```json { "packageManager": "yarn@https://registry.npmjs.org/@yarnpkg/cli-dist/-/cli-dist-3.2.3.tgz#sha224.16a0797d1710d1fb7ec40ab5c3801b68370a612a9b66ba117ad9924b" diff --git a/sources/binUtils.ts b/sources/binUtils.ts new file mode 100644 index 000000000..3060823a3 --- /dev/null +++ b/sources/binUtils.ts @@ -0,0 +1,64 @@ +import {UsageError} from 'clipanion'; +import fs from 'fs'; +import path from 'path'; + +import type {BinSpec} from './types.ts'; + +function isInside(folder: string, target: string) { + const relative = path.relative(folder, target); + return relative !== `` && relative !== `..` && !relative.startsWith(`..${path.sep}`) && !path.isAbsolute(relative); +} + +async function resolvePackageFile(folder: string, file: unknown, description: string) { + if (typeof file !== `string` || file.length === 0 || /[\\:\0]/.test(file) || path.posix.isAbsolute(file)) + throw new UsageError(`Invalid ${description} in package.json: expected a relative file path`); + + const target = path.resolve(folder, file); + if (!isInside(folder, target)) + throw new UsageError(`Invalid ${description} in package.json: path must stay inside the package`); + + let resolved: string; + try { + resolved = await fs.promises.realpath(target); + } catch { + throw new UsageError(`Unable to locate ${description} in the downloaded package`); + } + + if (!isInside(folder, resolved)) + throw new UsageError(`Invalid ${description} in package.json: path must stay inside the package`); + + if (!(await fs.promises.stat(resolved)).isFile()) + throw new UsageError(`Invalid ${description} in package.json: expected a file`); + + return path.relative(folder, resolved).split(path.sep).join(`/`); +} + +export async function readPackageBin(folder: string, allowedNames?: Array): Promise { + // Resolve the package root too: the cache itself may be reached through a symlink. + const packageRoot = await fs.promises.realpath(folder); + const manifestPath = await resolvePackageFile(packageRoot, `package.json`, `package manifest`); + const manifest = JSON.parse(await fs.promises.readFile(path.join(packageRoot, manifestPath), `utf8`)); + const packageBin = typeof manifest?.bin === `string` + ? typeof manifest.name === `string` && manifest.name.length > 0 + ? {[manifest.name.replace(/^@[^/]+\//, ``)]: manifest.bin} + : null + : manifest?.bin; + + if (typeof packageBin !== `object` || packageBin === null || Array.isArray(packageBin) || Object.keys(packageBin).length === 0) + throw new UsageError(`Unable to locate bin in package.json`); + + const bin: BinSpec = {}; + for (const name of allowedNames ?? Object.keys(packageBin)) { + // Optional aliases may disappear between releases. Never inherit an entry. + if (!Object.hasOwn(packageBin, name)) + continue; + + const file = await resolvePackageFile(packageRoot, packageBin[name], `bin '${name}'`); + Object.defineProperty(bin, name, {value: file, enumerable: true}); + } + + if (Object.keys(bin).length === 0) + throw new UsageError(`Unable to locate supported bin in package.json`); + + return bin; +} diff --git a/sources/corepackUtils.ts b/sources/corepackUtils.ts index d38df1c7b..196259140 100644 --- a/sources/corepackUtils.ts +++ b/sources/corepackUtils.ts @@ -11,6 +11,7 @@ import semverParse from 'semver import {setTimeout as setTimeoutPromise} from 'timers/promises'; import * as engine from './Engine.ts'; +import * as binUtils from './binUtils.ts'; import * as debugUtils from './debugUtils.ts'; import * as folderUtils from './folderUtils.ts'; import * as httpUtils from './httpUtils.ts'; @@ -303,18 +304,14 @@ export async function installVersion(installTarget: string, locator: Locator, {s bin = [locator.name]; } } else { - if (locatorIsASupportedPackageManager && isValidBinSpec(spec.bin)) { - bin = spec.bin; - } else { - const {name: packageName, bin: packageBin} = require(path.join(tmpFolder, `package.json`)); - if (typeof packageBin === `string`) { - // When `bin` is a string, the name of the executable is the name of the package. - bin = {[packageName]: packageBin}; - } else if (isValidBinSpec(packageBin)) { - bin = packageBin; - } else { - throw new Error(`Unable to locate bin in package.json`); - } + const allowedNames = locatorIsASupportedPackageManager + ? isValidBinList(spec.bin) ? spec.bin : isValidBinSpec(spec.bin) ? Object.keys(spec.bin) : undefined + : undefined; + try { + bin = await binUtils.readPackageBin(tmpFolder, allowedNames); + } catch (error) { + await fs.promises.rm(tmpFolder, {recursive: true, force: true}); + throw error; } } diff --git a/tests/PackageBin.test.ts b/tests/PackageBin.test.ts new file mode 100644 index 000000000..2e530c0c5 --- /dev/null +++ b/tests/PackageBin.test.ts @@ -0,0 +1,55 @@ +import type {Filename} from '@yarnpkg/fslib'; +import {npath, ppath, xfs} from '@yarnpkg/fslib'; +import {beforeEach, describe, expect, it} from 'vitest'; + +import {runCli} from './_runCli.ts'; + +beforeEach(async () => { + const home = await xfs.mktempPromise(); + process.env.COREPACK_HOME = npath.fromPortablePath(home); + process.env.COREPACK_DEFAULT_TO_LATEST = `0`; + process.env.AUTH_TYPE = `COREPACK_NPM_TOKEN`; + process.env.TEST_INTEGRITY = `valid`; + return () => xfs.removePromise(home, {recursive: true}); +}); + +describe(`tarball package executable paths`, () => { + it(`runs moved pnpm and pnpx entry points and reuses them offline`, async () => { + process.env.TEST_PNPM_BIN_LAYOUT = JSON.stringify({pnpm: `dist/pnpm.mjs`, pnpx: `dist/pnpx.mjs`, npm: `../outside.js`}); + await xfs.mktempPromise(async cwd => { + for (const name of [`pnpm`, `pnpx`]) { + await expect(runCli(cwd, [`${name}@11.9998.9999`, `--version`], true)).resolves.toEqual({ + exitCode: 0, stdout: `${name}: moved entry --version\n`, stderr: ``, + }); + } + const cache = ppath.join(npath.toPortablePath(process.env.COREPACK_HOME!), `v1/pnpm/11.9998.9999`); + await expect(xfs.readJsonPromise(ppath.join(cache, `.corepack` as Filename))).resolves.toMatchObject({ + bin: {pnpm: `dist/pnpm.mjs`, pnpx: `dist/pnpx.mjs`}, + hash: expect.stringMatching(/^sha512\.[a-f0-9]{128}$/), + }); + process.env.COREPACK_ENABLE_NETWORK = `0`; + await expect(runCli(cwd, [`pnpm@11.9998.9999`, `exec`, `hello`])).resolves.toEqual({ + exitCode: 0, stdout: `pnpm: moved entry exec hello\n`, stderr: ``, + }); + }); + }); + + it(`accepts string bins when optional aliases are absent`, async () => { + process.env.TEST_PNPM_BIN_LAYOUT = JSON.stringify(`dist/pnpm.mjs`); + await xfs.mktempPromise(async cwd => { + await expect(runCli(cwd, [`pnpm@11.9998.9999`, `--version`], true)).resolves.toEqual({ + exitCode: 0, stdout: `pnpm: moved entry --version\n`, stderr: ``, + }); + }); + }); + + it.each([`../outside.js`, `missing.js`, `dist`])(`rejects invalid entry '%s' without caching or running it`, async file => { + process.env.TEST_PNPM_BIN_LAYOUT = JSON.stringify({pnpm: file}); + await xfs.mktempPromise(async cwd => { + await expect(runCli(cwd, [`pnpm@11.9998.9999`, `--version`], true)).resolves.toMatchObject({ + exitCode: 1, stdout: ``, stderr: expect.stringContaining(`bin 'pnpm'`), + }); + await expect(xfs.readdirPromise(ppath.join(npath.toPortablePath(process.env.COREPACK_HOME!), `v1`))).resolves.toEqual([]); + }); + }); +}); diff --git a/tests/_registryServer.mjs b/tests/_registryServer.mjs index cb4553d04..22f1bae5f 100644 --- a/tests/_registryServer.mjs +++ b/tests/_registryServer.mjs @@ -61,7 +61,13 @@ function createSimpleTarArchive(fileName, fileContent, mode = 0o644) { ]); } -const mockPackageTarGz = gzipSync(Buffer.concat([ +const changedPnpmBin = process.env.TEST_PNPM_BIN_LAYOUT; +const mockPackageTarGz = gzipSync(Buffer.concat(changedPnpmBin ? [ + createSimpleTarArchive(`package/dist/pnpm.mjs`, `#!/usr/bin/env node\nconsole.log("pnpm: moved entry " + process.argv.slice(2).join(" "));\n`, 0o755), + createSimpleTarArchive(`package/dist/pnpx.mjs`, `#!/usr/bin/env node\nconsole.log("pnpx: moved entry " + process.argv.slice(2).join(" "));\n`, 0o755), + createSimpleTarArchive(`package/package.json`, JSON.stringify({name: `pnpm`, type: `module`, bin: JSON.parse(changedPnpmBin)})), + Buffer.alloc(1024), +] : [ createSimpleTarArchive(`package/bin/customPkgManager.js`, `#!/usr/bin/env node\nconsole.log("customPkgManager: Hello from custom registry");\n`, 0o755), createSimpleTarArchive(`package/bin/pnpm.js`, `#!/usr/bin/env node\nconsole.log("pnpm: Hello from custom registry");\n`, 0o755), createSimpleTarArchive(`package/bin/yarn.js`, `#!/usr/bin/env node\nconsole.log("yarn: Hello from custom registry");\n`, 0o755), @@ -78,7 +84,7 @@ const integrity = `sha512-${createHash(`sha512`).update( const registry = { __proto__: null, yarn: [`1.9998.9999`], - pnpm: [`1.9998.9999`], + pnpm: changedPnpmBin ? [`11.9998.9999`] : [`1.9998.9999`], '@yarnpkg/cli-dist': [`5.9999.9999`], customPkgManager: [`1.0.0`], diff --git a/tests/binUtils.test.ts b/tests/binUtils.test.ts new file mode 100644 index 000000000..f9d2ce8ce --- /dev/null +++ b/tests/binUtils.test.ts @@ -0,0 +1,140 @@ +import fs from 'fs'; +import os from 'os'; +import path from 'path'; +import {beforeEach, describe, expect, it} from 'vitest'; + +import {readPackageBin} from '../sources/binUtils.ts'; + +let folder: string; + +beforeEach(async () => { + folder = await fs.promises.mkdtemp(path.join(os.tmpdir(), `corepack-bin-`)); + await fs.promises.mkdir(path.join(folder, `dist`)); + await fs.promises.writeFile(path.join(folder, `dist`, `pnpm.mjs`), `export {};`); + return () => fs.promises.rm(folder, {recursive: true, force: true}); +}); + +async function writeManifest(manifest: unknown) { + await fs.promises.writeFile(path.join(folder, `package.json`), JSON.stringify(manifest)); +} + +describe(`package executable paths`, () => { + it(`reads changed paths and keeps only supported commands`, async () => { + await writeManifest({bin: {pnpm: `./dist/pnpm.mjs`, pnpx: `dist/pnpm.mjs`, npm: `../outside.js`}}); + await expect(readPackageBin(folder, [`pnpm`, `pnpx`])).resolves.toEqual({pnpm: `dist/pnpm.mjs`, pnpx: `dist/pnpm.mjs`}); + }); + + it(`omits aliases no longer published by the package`, async () => { + await writeManifest({bin: {pnpm: `dist/pnpm.mjs`}}); + await expect(readPackageBin(folder, [`pnpm`, `pnpx`])).resolves.toEqual({pnpm: `dist/pnpm.mjs`}); + }); + + it.each([`pnpm`, `@scope/pnpm`])(`uses the unscoped package name for string bins (%s)`, async name => { + await writeManifest({name, bin: `dist/pnpm.mjs`}); + await expect(readPackageBin(folder, [`pnpm`, `pnpx`])).resolves.toEqual({pnpm: `dist/pnpm.mjs`}); + }); + + it(`supports custom command names without a configured allowlist`, async () => { + await writeManifest({bin: {custom: `dist/pnpm.mjs`}}); + await expect(readPackageBin(folder)).resolves.toEqual({custom: `dist/pnpm.mjs`}); + }); + + it(`does not use inherited properties as aliases`, async () => { + await writeManifest({bin: {pnpm: `dist/pnpm.mjs`}}); + await expect(readPackageBin(folder, [`pnpm`, `toString`, `constructor`])).resolves.toEqual({pnpm: `dist/pnpm.mjs`}); + }); + + it(`keeps custom __proto__ entries as own data properties`, async () => { + await writeManifest({bin: JSON.parse(`{"__proto__":"dist/pnpm.mjs"}`)}); + const bin = await readPackageBin(folder); + expect(Object.keys(bin)).toEqual([`__proto__`]); + expect(Object.getPrototypeOf(bin)).toBe(Object.prototype); + }); + + it.each([undefined, null, [], {}, 123, true])(`rejects invalid bin declarations (%j)`, async bin => { + await writeManifest({bin}); + await expect(readPackageBin(folder)).rejects.toThrow(`Unable to locate bin in package.json`); + }); + + it(`rejects a string bin without a package name`, async () => { + await writeManifest({bin: `dist/pnpm.mjs`}); + await expect(readPackageBin(folder)).rejects.toThrow(`Unable to locate bin in package.json`); + }); + + it(`rejects a manifest without any supported commands`, async () => { + await writeManifest({bin: {npm: `dist/pnpm.mjs`}}); + await expect(readPackageBin(folder, [`pnpm`, `pnpx`])).rejects.toThrow(`Unable to locate supported bin`); + }); + + it.each([``, null, 123, {}, [], `/tmp/outside.js`, `C:/outside.js`, `C:outside.js`, `\\\\server\\outside.js`, `..\\outside.js`, `dist/pnpm.mjs\0`])(`rejects invalid entry paths (%j)`, async file => { + await writeManifest({bin: {pnpm: file}}); + await expect(readPackageBin(folder)).rejects.toThrow(`expected a relative file path`); + }); + + it.each([`../outside.js`, `dist/../../outside.js`, `.`])(`rejects paths escaping or naming the package root (%s)`, async file => { + await writeManifest({bin: {pnpm: file}}); + await expect(readPackageBin(folder)).rejects.toThrow(`path must stay inside the package`); + }); + + it(`rejects sibling paths that share the package directory prefix`, async () => { + await writeManifest({bin: {pnpm: `../${path.basename(folder)}-other/cli.js`}}); + await expect(readPackageBin(folder)).rejects.toThrow(`path must stay inside the package`); + }); + + it(`rejects missing executable files`, async () => { + await writeManifest({bin: {pnpm: `missing.js`}}); + await expect(readPackageBin(folder)).rejects.toThrow(`Unable to locate bin 'pnpm'`); + }); + + it(`rejects directories as entry points`, async () => { + await writeManifest({bin: {pnpm: `dist`}}); + await expect(readPackageBin(folder)).rejects.toThrow(`expected a file`); + }); + + it(`rejects malformed manifest JSON`, async () => { + await fs.promises.writeFile(path.join(folder, `package.json`), `{`); + await expect(readPackageBin(folder)).rejects.toThrow(SyntaxError); + }); + + it(`rejects a missing package manifest`, async () => { + await expect(readPackageBin(folder)).rejects.toThrow(`Unable to locate package manifest`); + }); + + // Windows permits directory junctions without the privileges required for file symlinks. + it(`accepts symlinks contained in the package and caches the resolved path`, async () => { + await fs.promises.symlink(path.join(folder, `dist`), path.join(folder, `alias`), `junction`); + await writeManifest({bin: {pnpm: `alias/pnpm.mjs`}}); + await expect(readPackageBin(folder)).resolves.toEqual({pnpm: `dist/pnpm.mjs`}); + }); + + it(`accepts a package root reached through a symlink`, async () => { + await fs.promises.symlink(path.join(folder, `dist`), path.join(folder, `alias`), `junction`); + await fs.promises.writeFile(path.join(folder, `dist`, `package.json`), JSON.stringify({bin: {pnpm: `pnpm.mjs`}})); + await expect(readPackageBin(path.join(folder, `alias`))).resolves.toEqual({pnpm: `pnpm.mjs`}); + }); + + it(`rejects symlinked entry points outside the package`, async () => { + await fs.promises.symlink(path.dirname(folder), path.join(folder, `outside`), `junction`); + // Use a separate sibling package file so realpath cannot return inside this package. + const outside = await fs.promises.mkdtemp(path.join(path.dirname(folder), `corepack-bin-outside-`)); + try { + await fs.promises.writeFile(path.join(outside, `cli.js`), ``); + await writeManifest({bin: {pnpm: `outside/${path.basename(outside)}/cli.js`}}); + await expect(readPackageBin(folder)).rejects.toThrow(`path must stay inside the package`); + } finally { + await fs.promises.rm(outside, {recursive: true, force: true}); + } + }); + + it(`rejects package manifests symlinked outside the package`, async () => { + const outside = await fs.promises.mkdtemp(path.join(path.dirname(folder), `corepack-manifest-outside-`)); + try { + await fs.promises.writeFile(path.join(outside, `package.json`), JSON.stringify({bin: {pnpm: `dist/pnpm.mjs`}})); + // A manifest file symlink requires privileges on Windows; a directory named package.json is still invalid. + await fs.promises.symlink(outside, path.join(folder, `package.json`), `junction`); + await expect(readPackageBin(folder)).rejects.toThrow(`path must stay inside the package`); + } finally { + await fs.promises.rm(outside, {recursive: true, force: true}); + } + }); +});