From 45fd581c23b51fca025d716ae42cf4eda0122d9f Mon Sep 17 00:00:00 2001 From: Dan Lynch Date: Fri, 4 Sep 2026 01:48:48 +0000 Subject: [PATCH] =?UTF-8?q?feat(pnpm-policy):=20denyBuilds=20=E2=80=94=20r?= =?UTF-8?q?ecord=20install=20scripts=20that=20were=20reviewed=20and=20are?= =?UTF-8?q?=20not=20needed?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pnpm 11 fails the install (ERR_PNPM_IGNORED_BUILDS) for any build script that is neither allowed nor denied, so a policy that can only allow leaves fresh clones stuck on the interactive pnpm approve-builds prompt. denyBuilds emits false entries into the allowBuilds map (ignoredBuiltDependencies for the legacy key), with reasons, so the decision lives in pnpm-policy.yaml. Also close the three open questions in this workspace's own pnpm-workspace.yaml so pnpm 11 installs cleanly. --- packages/pnpm-policy/README.md | 13 +++++ packages/pnpm-policy/__tests__/policy.test.ts | 37 ++++++++++++ packages/pnpm-policy/src/cli.ts | 8 ++- packages/pnpm-policy/src/config.ts | 34 +++++++++-- packages/pnpm-policy/src/generate.ts | 4 +- packages/pnpm-policy/src/index.ts | 9 +-- packages/pnpm-policy/src/policy.ts | 58 +++++++++++++++---- packages/pnpm-policy/src/types.ts | 13 +++++ pnpm-workspace.yaml | 6 ++ 9 files changed, 159 insertions(+), 23 deletions(-) diff --git a/packages/pnpm-policy/README.md b/packages/pnpm-policy/README.md index 7297fb10..3f5526e8 100644 --- a/packages/pnpm-policy/README.md +++ b/packages/pnpm-policy/README.md @@ -105,6 +105,7 @@ settings: | `inventory` | path, package, or list | – | Where the inventory comes from. A list is merged. | | `intersect` | boolean | `true` | Only emit names this workspace actually resolves. | | `allowBuilds` | map or list | `{}` | Dependencies permitted to run install scripts. | +| `denyBuilds` | map or list | `{}` | Dependencies whose install scripts were reviewed and are not needed. | | `exceptions` | list | `[]` | Third-party bypasses, each with a reason. | | `settings` | map | `{}` | Extra pnpm settings to include in the managed block. | @@ -170,6 +171,18 @@ That becomes pnpm's `allowBuilds` map (pnpm ≥ 10.16), with the reasons as inli Unlike the release-age exemptions, this list is **not** derived from anything: a package that runs install scripts is a deliberate trust decision, whoever published it. +### `denyBuilds` + +pnpm treats a dependency with an install script that appears in neither list as an open question: it warns, and pnpm 11 fails the install with `ERR_PNPM_IGNORED_BUILDS` until someone runs the interactive `pnpm approve-builds`. Most such scripts are fallbacks — `nx`, `@parcel/watcher` and `unrs-resolver` all ship prebuilt binaries as optional dependencies and only compile when none fits. Close the question in the policy, with the reason, instead of in a prompt: + +```yaml +denyBuilds: + nx: prebuilt binary ships as an optional dep + "@parcel/watcher": prebuilt binary ships as an optional dep +``` + +These become `false` entries in the same `allowBuilds` map (or, with `--builds-key onlyBuiltDependencies`, an `ignoredBuiltDependencies` array). A name in both lists is an error. Do not run `pnpm approve-builds` in a workspace managed by pnpm-policy: it writes an `allowBuilds` entry that the next `generate` overwrites and that `check` reports as drift. + ## Understanding what you depend on Deciding what to exempt means deciding which *projects* you trust, but npm only offers accounts — and an account is as wide as everything its owner will ever publish. The person maintaining a library you want may also co-maintain something enormous you did not mean to exempt. diff --git a/packages/pnpm-policy/__tests__/policy.test.ts b/packages/pnpm-policy/__tests__/policy.test.ts index 6edcb1b2..c95f5845 100644 --- a/packages/pnpm-policy/__tests__/policy.test.ts +++ b/packages/pnpm-policy/__tests__/policy.test.ts @@ -117,6 +117,43 @@ describe('resolvePolicy', () => { expect(policy.settings.allowBuilds).toBeUndefined(); }); + it('writes denyBuilds as false entries in the same allowBuilds map', () => { + const { settings, comments } = resolve({ + allowBuilds: { esbuild: 'native binary' }, + denyBuilds: { nx: 'prebuilt binary ships as an optional dep', '@parcel/watcher': 'same' } + }); + expect(settings.allowBuilds).toEqual({ + '@parcel/watcher': false, + esbuild: true, + nx: false + }); + expect(commentAt(comments.inline, ['allowBuilds', 'nx'])).toBe( + 'prebuilt binary ships as an optional dep' + ); + expect(commentAt(comments.before, ['allowBuilds'])).toMatch(/false means reviewed/); + }); + + it('emits a map when only denyBuilds is set', () => { + const { settings } = resolve({ denyBuilds: ['nx'] }); + expect(settings.allowBuilds).toEqual({ nx: false }); + }); + + it('writes denyBuilds as ignoredBuiltDependencies for older pnpm', () => { + const policy = resolvePolicy({ + config: normalizeConfig({ allowBuilds: ['esbuild'], denyBuilds: ['nx', '@parcel/watcher'] }), + buildsKey: 'onlyBuiltDependencies' + }); + expect(policy.settings.onlyBuiltDependencies).toEqual(['esbuild']); + expect(policy.settings.ignoredBuiltDependencies).toEqual(['@parcel/watcher', 'nx']); + expect(policy.settings.allowBuilds).toBeUndefined(); + }); + + it('rejects a package that is both allowed and denied', () => { + expect(() => normalizeConfig({ allowBuilds: ['nx'], denyBuilds: ['nx'] })).toThrow( + /both allowBuilds and denyBuilds/ + ); + }); + it('passes extra settings through verbatim', () => { const { settings } = resolve({ settings: { trustPolicy: 'strict' } }); expect(settings.trustPolicy).toBe('strict'); diff --git a/packages/pnpm-policy/src/cli.ts b/packages/pnpm-policy/src/cli.ts index e554005a..17710af7 100644 --- a/packages/pnpm-policy/src/cli.ts +++ b/packages/pnpm-policy/src/cli.ts @@ -6,7 +6,7 @@ import { findConfig, loadConfig } from './config'; import { formatDuration } from './duration'; import { PolicyError } from './errors'; import { check, generate } from './generate'; -import { readWorkspaceGraph, reachableFrom } from './graph'; +import { reachableFrom,readWorkspaceGraph } from './graph'; import { buildInventory, writeInventory } from './inventory'; import { readWorkspacePackages } from './lockfile'; import { groupByOwner, namesFromOwners, packageOrigins } from './origins'; @@ -70,6 +70,12 @@ inventory: ./pnpm-policy.inventory.json allowBuilds: esbuild: native binary, downloaded at install time +# Dependencies whose install scripts you have looked at and do not need. Without +# a decision here pnpm warns (or, on pnpm 11, refuses to install) until someone +# runs \`pnpm approve-builds\` by hand. +denyBuilds: {} +# nx: prebuilt binary ships as an optional dep + # Third-party escape hatches. A reason is required; \`until\` makes the waiver # expire so \`pnpm-policy check\` reminds you to re-justify or remove it. exceptions: [] diff --git a/packages/pnpm-policy/src/config.ts b/packages/pnpm-policy/src/config.ts index 59e75888..b025123a 100644 --- a/packages/pnpm-policy/src/config.ts +++ b/packages/pnpm-policy/src/config.ts @@ -8,7 +8,13 @@ import { parse as parseYaml } from 'yaml'; import { parseDuration } from './duration'; import { PolicyError } from './errors'; -import type { AllowedBuild, PolicyConfig, PolicyException, ResolvedConfig } from './types'; +import type { + AllowedBuild, + DeniedBuild, + PolicyConfig, + PolicyException, + ResolvedConfig +} from './types'; /** Filenames searched for, in order, when no explicit path is given. */ export const CONFIG_FILENAMES = ['pnpm-policy.yaml', 'pnpm-policy.yml', 'pnpm-policy.json']; @@ -39,10 +45,13 @@ export function readConfig(file: string): PolicyConfig { return parsed; } -function normalizeAllowBuilds(input: PolicyConfig['allowBuilds']): AllowedBuild[] { +function normalizeBuilds( + key: 'allowBuilds' | 'denyBuilds', + input: PolicyConfig['allowBuilds'] | PolicyConfig['denyBuilds'] +): Array { if (!input) return []; - const builds: AllowedBuild[] = Array.isArray(input) + const builds: Array = Array.isArray(input) ? input.map((entry) => typeof entry === 'string' ? { package: entry } : { ...entry } ) @@ -53,7 +62,7 @@ function normalizeAllowBuilds(input: PolicyConfig['allowBuilds']): AllowedBuild[ for (const build of builds) { if (!build.package) { - throw new PolicyError('An allowBuilds entry is missing a package name'); + throw new PolicyError(`A ${key} entry is missing a package name`); } } @@ -61,6 +70,17 @@ function normalizeAllowBuilds(input: PolicyConfig['allowBuilds']): AllowedBuild[ return builds.sort((a, b) => a.package.localeCompare(b.package)); } +function checkBuildConflicts(allow: AllowedBuild[], deny: DeniedBuild[]): void { + const denied = new Set(deny.map((build) => build.package)); + for (const build of allow) { + if (denied.has(build.package)) { + throw new PolicyError( + `"${build.package}" is listed in both allowBuilds and denyBuilds; pick one` + ); + } + } +} + function normalizeExceptions(input: PolicyException[] | undefined): PolicyException[] { const exceptions = input ?? []; for (const exception of exceptions) { @@ -107,6 +127,9 @@ function normalizeInventory(value: string | string[] | undefined): string[] { /** Apply defaults and convert a config into the shape the resolver consumes. */ export function normalizeConfig(config: PolicyConfig): ResolvedConfig { + const allowBuilds = normalizeBuilds('allowBuilds', config.allowBuilds); + const denyBuilds = normalizeBuilds('denyBuilds', config.denyBuilds); + checkBuildConflicts(allowBuilds, denyBuilds); return { minimumReleaseAgeMinutes: parseDuration( config.minimumReleaseAge ?? DEFAULT_MINIMUM_RELEASE_AGE @@ -116,7 +139,8 @@ export function normalizeConfig(config: PolicyConfig): ResolvedConfig { scopes: normalizeScopes(config.scopes), inventory: normalizeInventory(config.inventory), intersect: config.intersect ?? true, - allowBuilds: normalizeAllowBuilds(config.allowBuilds), + allowBuilds, + denyBuilds, exceptions: normalizeExceptions(config.exceptions), settings: config.settings ?? {} }; diff --git a/packages/pnpm-policy/src/generate.ts b/packages/pnpm-policy/src/generate.ts index ba19decd..136dee25 100644 --- a/packages/pnpm-policy/src/generate.ts +++ b/packages/pnpm-policy/src/generate.ts @@ -67,8 +67,8 @@ function load(options: RunOptions): Loaded { // to trust — without flattening them into a copy checked in beside the config. const inventory = config.inventory.length ? mergeInventories( - config.inventory.map((reference) => loadOneInventory(configFile, reference)) - ) + config.inventory.map((reference) => loadOneInventory(configFile, reference)) + ) : undefined; if (!inventory && config.maintainers.length && !config.scopes.length) { diff --git a/packages/pnpm-policy/src/index.ts b/packages/pnpm-policy/src/index.ts index da8f25e4..89261864 100644 --- a/packages/pnpm-policy/src/index.ts +++ b/packages/pnpm-policy/src/index.ts @@ -12,9 +12,7 @@ export { PolicyError } from './errors'; export type { CheckResult, GenerateResult, RunOptions } from './generate'; export { check, generate } from './generate'; export type { DependencyGraph } from './graph'; -export { readLockfileGraph, readWorkspaceGraph, reachableFrom } from './graph'; -export type { PackageOrigin } from './origins'; -export { groupByOwner, namesFromOwners, packageOrigins, repositorySlug } from './origins'; +export { reachableFrom,readLockfileGraph, readWorkspaceGraph } from './graph'; export type { BuildInventoryOptions } from './inventory'; export { buildInventory, @@ -31,8 +29,10 @@ export { readLockfilePackages, readWorkspacePackages } from './lockfile'; +export type { PackageOrigin } from './origins'; +export { groupByOwner, namesFromOwners, packageOrigins, repositorySlug } from './origins'; export type { BuildsKey, ResolveOptions } from './policy'; -export { exceptionPattern, managedKeys, resolvePolicy } from './policy'; +export { exceptionPattern, IGNORED_BUILDS_KEY, managedKeys, resolvePolicy } from './policy'; export type { RegistryOptions } from './registry'; export { DEFAULT_REGISTRY, @@ -42,6 +42,7 @@ export { } from './registry'; export type { AllowedBuild, + DeniedBuild, Duration, Inventory, PolicyConfig, diff --git a/packages/pnpm-policy/src/policy.ts b/packages/pnpm-policy/src/policy.ts index 7a364455..0061c7f4 100644 --- a/packages/pnpm-policy/src/policy.ts +++ b/packages/pnpm-policy/src/policy.ts @@ -20,6 +20,9 @@ import type { /** Which pnpm key carries the build allowlist. */ export type BuildsKey = 'allowBuilds' | 'onlyBuiltDependencies'; +/** The pre-10.16 companion to `onlyBuiltDependencies`, carrying the denials. */ +export const IGNORED_BUILDS_KEY = 'ignoredBuiltDependencies'; + export interface ResolveOptions { config: ResolvedConfig; inventory?: Inventory; @@ -105,8 +108,8 @@ function buildComments( `Exempt from the wait: ${sources.join(', ')}.`, config.maintainers.length ? `First-party membership comes from what ${config.maintainers.join(', ')} ${ - config.maintainers.length === 1 ? 'publishes' : 'publish' - } on npm — waiting on your own release protects nothing.` + config.maintainers.length === 1 ? 'publishes' : 'publish' + } on npm — waiting on your own release protects nothing.` : report.firstPartyPackages.length ? 'First-party membership comes from the inventory.' : 'First-party membership comes from the scopes claimed in pnpm-policy.yaml.' @@ -124,12 +127,31 @@ function buildComments( index++; } - if (config.allowBuilds.length) { - before.push([[buildsKey], 'The only dependencies permitted to run install scripts.']); + if (buildsKey === 'allowBuilds') { + if (config.allowBuilds.length || config.denyBuilds.length) { + before.push([ + [buildsKey], + config.denyBuilds.length + ? 'Install scripts: true runs them, false means reviewed and not needed.' + : 'The only dependencies permitted to run install scripts.' + ]); + } + for (const build of [...config.allowBuilds, ...config.denyBuilds]) { + if (build.reason) inline.push([[buildsKey, build.package], build.reason]); + } + } else { + if (config.allowBuilds.length) { + before.push([[buildsKey], 'The only dependencies permitted to run install scripts.']); + } config.allowBuilds.forEach((build, i) => { - if (!build.reason) return; - inline.push([[buildsKey, buildsKey === 'allowBuilds' ? build.package : i], build.reason]); + if (build.reason) inline.push([[buildsKey, i], build.reason]); }); + if (config.denyBuilds.length) { + before.push([[IGNORED_BUILDS_KEY], 'Install scripts reviewed and not needed.']); + config.denyBuilds.forEach((build, i) => { + if (build.reason) inline.push([[IGNORED_BUILDS_KEY, i], build.reason]); + }); + } } before.push([ @@ -170,11 +192,24 @@ export function resolvePolicy(options: ResolveOptions): ResolvedPolicy { if (exclude.length) { settings.minimumReleaseAgeExclude = exclude; } - if (config.allowBuilds.length) { - settings[buildsKey] = - buildsKey === 'allowBuilds' - ? Object.fromEntries(config.allowBuilds.map((build) => [build.package, true])) - : config.allowBuilds.map((build) => build.package); + if (buildsKey === 'allowBuilds') { + if (config.allowBuilds.length || config.denyBuilds.length) { + // One map, sorted by name, so allowed and denied entries interleave the + // way a reader scanning for a package expects. + settings.allowBuilds = Object.fromEntries( + [ + ...config.allowBuilds.map((build): [string, boolean] => [build.package, true]), + ...config.denyBuilds.map((build): [string, boolean] => [build.package, false]) + ].sort(([a], [b]) => a.localeCompare(b)) + ); + } + } else { + if (config.allowBuilds.length) { + settings.onlyBuiltDependencies = config.allowBuilds.map((build) => build.package); + } + if (config.denyBuilds.length) { + settings[IGNORED_BUILDS_KEY] = config.denyBuilds.map((build) => build.package); + } } settings.blockExoticSubdeps = config.blockExoticSubdeps; Object.assign(settings, config.settings); @@ -192,6 +227,7 @@ export function managedKeys(buildsKey: BuildsKey, extra: string[] = []): string[ 'minimumReleaseAge', 'minimumReleaseAgeExclude', buildsKey, + ...(buildsKey === 'onlyBuiltDependencies' ? [IGNORED_BUILDS_KEY] : []), 'blockExoticSubdeps', ...extra ]; diff --git a/packages/pnpm-policy/src/types.ts b/packages/pnpm-policy/src/types.ts index 0359c7ab..af5983df 100644 --- a/packages/pnpm-policy/src/types.ts +++ b/packages/pnpm-policy/src/types.ts @@ -23,6 +23,12 @@ export interface AllowedBuild { reason?: string; } +/** A dependency whose install scripts were reviewed and deliberately left off. */ +export interface DeniedBuild { + package: string; + reason?: string; +} + export interface PolicyConfig { /** * How long a third-party release must exist before it may be installed. @@ -55,6 +61,12 @@ export interface PolicyConfig { intersect?: boolean; /** Dependencies allowed to run install scripts. */ allowBuilds?: Array | Record; + /** + * Dependencies whose install scripts are known and not needed. pnpm treats an + * unlisted script as an open question — it warns, or on pnpm 11 fails the + * install and asks for `pnpm approve-builds` — so a `false` here closes it. + */ + denyBuilds?: Array | Record; /** Third-party escape hatches. */ exceptions?: PolicyException[]; /** Extra pnpm settings written verbatim into the workspace file. */ @@ -71,6 +83,7 @@ export interface ResolvedConfig { inventory: string[]; intersect: boolean; allowBuilds: AllowedBuild[]; + denyBuilds: DeniedBuild[]; exceptions: PolicyException[]; settings: Record; } diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index 18ec407e..9fce7392 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -1,2 +1,8 @@ packages: - 'packages/*' + +# Install scripts: true runs them, false means reviewed and not needed. +allowBuilds: + '@launchql/protobufjs': false # postinstall only prints a version-scheme notice + nx: false # prebuilt binary ships as an optional dep + unrs-resolver: false # prebuilt binary ships as an optional dep