fix: validate published bins and npm modes

.changeset/validate-published-bins.md

@@ -0,0 +1,5 @@
+---
+"agent-bundle": patch
+---
+
+Validate published package bins with AB7012 and restore npm-normalized executable modes without weakening AB7001 byte and digest checks. (#674)

packages/agent-bundle/src/api.ts

@@ -14,6 +14,7 @@ import { routedCliBins } from './build/cli-bins.ts';
 import { planComposite } from './build/compose.ts';
 import { buildPackageOutputs, type PackageBuildResult } from './build/package-build.ts';
 import {
+  packageBinDiagnostics,
   packInventoryDiagnostics,
   packOutputFromJson,
   type PackOutput,
@@ -1375,13 +1376,24 @@ export const prepack = async (options: BuildOptions): Promise<PrepackResult> =>
     cwd: result.packageBuild.outputRoot,
   });
   const pack = packOutputFromJson(stdout);
-  const diagnostics = await packInventoryDiagnostics({
+  const diagnostics = [...await packInventoryDiagnostics({
     model: result.model,
     packageBuild: result.packageBuild,
     packOutput: pack,
     packerRewritesWorkspaceProtocols: false,
     projectRoot: options.root,
-  });
+  })];
+  if (resolve(options.root) !== resolve(result.packageBuild.outputRoot)) {
+    const published = await execFile('npm', ['pack', '--dry-run', '--json', '--ignore-scripts'], {
+      cwd: options.root,
+    });
+    diagnostics.push(...await packageBinDiagnostics(
+      options.root,
+      packOutputFromJson(published.stdout),
+      'published package.json',
+    ));
+  }
+  diagnostics.sort((left, right) => left.code.localeCompare(right.code));
   if (hasErrors(diagnostics)) throw new DiagnosticError(diagnostics);
   return deepFreeze({ build: result, diagnostics, pack });
 };

packages/agent-bundle/src/build/pack-inventory.ts

@@ -15,7 +15,12 @@ import {
 } from './compile-evidence.ts';
 import { artifactManifestName } from './emit.ts';
 import { parseArtifactManifest } from './manifest.ts';
-import { declaredDependencies, type DeclaredDependency, type InstalledDependencyField } from '../core/package-dependencies.ts';
+import {
+  declaredDependencies,
+  packageBinEntries,
+  type DeclaredDependency,
+  type InstalledDependencyField,
+} from '../core/package-dependencies.ts';
 import {
   classifyDependency,
   installScriptDependencies,
@@ -104,19 +109,31 @@ const jsonRecord = async (path: string): Promise<Readonly<Record<string, unknown
   return value;
 };
 
-const binEntries = (value: unknown): readonly [string, string][] => {
-  if (typeof value === 'string') return Object.freeze([['bin', value] as const]);
-  if (!isRecord(value)) return Object.freeze([]);
-  return Object.freeze(Object.entries(value)
-    .filter((entry): entry is [string, string] => typeof entry[1] === 'string')
-    .sort(([left], [right]) => left.localeCompare(right)));
-};
-
 const diagnostic = (code: string, message: string, recovery: string, severity: DiagnosticSeverity = 'error'): Diagnostic =>
   Object.freeze({ code, message, recovery, severity });
 
 const quoteAll = (values: readonly string[]): string => values.map((value) => JSON.stringify(value)).join(', ');
 
+export const packageBinDiagnostics = async (
+  packageRoot: string,
+  packOutput: PackOutput,
+  source = 'generated npm-root package.json',
+): Promise<readonly Diagnostic[]> => {
+  const packageDocument = await jsonRecord(join(packageRoot, 'package.json'));
+  const packed = new Set(packOutput.files.map((file) => file.path.replace(/^\.\//u, '')));
+  const invalidBins = packageBinEntries(packageDocument)
+    .filter(([, target]) => {
+      const normalized = target.replace(/^\.\//u, '');
+      return normalized.startsWith('src/') || !packed.has(normalized);
+    });
+  return invalidBins.length === 0 ? [] : [diagnostic(
+    'AB7012',
+    `${source} bins must name files in its packed file set: ${invalidBins.map(([name, target]) =>
+      `${JSON.stringify(name)} -> ${JSON.stringify(target)}`).join(', ')}.`,
+    'Point each bin at a file included by that package; routed CLIs use manifest-declared bin/<name>.mjs and authored bins use generated bin/*.js files.',
+  )];
+};
+
 /** One diagnostic per installed-dependency field that has offending entries, entries sorted by name. */
 const perField = (
   entries: readonly DeclaredDependency[],
@@ -297,19 +314,7 @@ export const packInventoryDiagnostics = async (options: {
     ));
   }
 
-  const invalidBins = binEntries(packageDocument.bin)
-    .filter(([, target]) => {
-      const normalized = target.replace(/^\.\//u, '');
-      return normalized.startsWith('src/') || !packed.has(normalized);
-    });
-  if (invalidBins.length > 0) {
-    diagnostics.push(diagnostic(
-      'AB7012',
-      `package.json bins must name files in the packed npm root: ${invalidBins.map(([name, target]) =>
-        `${JSON.stringify(name)} -> ${JSON.stringify(target)}`).join(', ')}.`,
-      'Point routed CLIs at their manifest-declared bin/<name>.mjs and authored bins at generated bin/*.js files.',
-    ));
-  }
+  diagnostics.push(...await packageBinDiagnostics(packageRoot, options.packOutput));
 
   const versions: Array<readonly [string, unknown]> = [
     ['package.json', packageDocument.version],

packages/agent-bundle/src/core/package-dependencies.ts

@@ -36,6 +36,16 @@ export interface DeclaredDependency {
   readonly installed: boolean;
 }
 
+export const packageBinEntries = (
+  packageDocument: Readonly<Record<string, unknown>>,
+): readonly (readonly [string, string])[] => {
+  if (typeof packageDocument.bin === 'string') return Object.freeze([['bin', packageDocument.bin] as const]);
+  if (!isRecord(packageDocument.bin)) return Object.freeze([]);
+  return Object.freeze(Object.entries(packageDocument.bin)
+    .filter((entry): entry is [string, string] => typeof entry[1] === 'string')
+    .sort(([left], [right]) => left.localeCompare(right)));
+};
+
 /** Peers `peerDependenciesMeta` marks optional: npm parses but never installs them. */
 const optionalPeers = (packageDocument: Readonly<Record<string, unknown>>): ReadonlySet<string> => {
   const meta = packageDocument.peerDependenciesMeta;

packages/agent-bundle/src/install/identity.ts

@@ -125,9 +125,12 @@ export const readBundleIdentity = async (
 };
 
 /** Reads and verifies the artifact-side inventory, preserving AB7001 as the bundle contract. */
-export const bundleInventory = async (identity: PluginIdentity): Promise<TreeInventory> => {
+export const bundleInventory = async (
+  identity: PluginIdentity,
+  options: { readonly restoreModes?: boolean } = {},
+): Promise<TreeInventory> => {
   try {
-    return await manifestInventory(identity.bundleRoot, identity.manifest);
+    return await manifestInventory(identity.bundleRoot, identity.manifest, options);
   } catch (error) {
     if (error instanceof DiagnosticError) throw error;
     throw failure('AB7001', errorMessage(error), identity.host);
@@ -150,7 +153,10 @@ export const installedBundleInventory = async (
         host,
       );
     case 'ok':
-      return manifestInventory(read.root, read.manifest, { verifyHashes: false });
+      return manifestInventory(read.root, read.manifest, {
+        hashManifestModes: false,
+        verifyHashes: false,
+      });
     default: {
       const exhaustive: never = read;
       throw new TypeError(`Unknown artifact manifest read result ${String(exhaustive)}.`);

packages/agent-bundle/src/install/install.ts

@@ -461,7 +461,7 @@ const installPublicCli = async (
   const environment = options.environment ?? process.env;
   const home = options.home ?? homedir();
   const id = `${identity.plugin}@${marketplace}`;
-  const artifact = await bundleInventory(identity);
+  const artifact = await bundleInventory(identity, { restoreModes: true });
   const inventory = await readPublicHostInventory(runner, identity, host, scope, environment, home);
   if (inventory.status === 'unavailable' && options.replace === true) {
     throw failure(
@@ -749,7 +749,7 @@ const installCursorMarketplace = async (
 ): Promise<InstallResult> => {
   const cursorRoot = await resolveCursorRoot(options);
   try {
-    const artifact = await bundleInventory(identity);
+    const artifact = await bundleInventory(identity, { restoreModes: true });
     const staged = await stageCursorMarketplace({
       artifact,
       cursorRoot,
@@ -911,7 +911,7 @@ const installCursor = Effect.fnUntraced(function*(
     version: identity.version,
   } as const;
   const program = Effect.gen(function*() {
-    const artifact = yield* liftPromise(() => bundleInventory(identity));
+    const artifact = yield* liftPromise(() => bundleInventory(identity, { restoreModes: true }));
     // The receipt records which host directories this installer created on the way to the plugin root
     // (a fresh Cursor home has no `plugins/local`), so uninstall can prune exactly those and no more.
     const hostDirectories: string[] = [];

packages/agent-bundle/src/install/receipt.ts

@@ -1,6 +1,7 @@
 import { createHash, randomUUID } from 'node:crypto';
 import type { Stats } from 'node:fs';
 import {
+  chmod,
   cp,
   lstat,
   mkdir,
@@ -18,6 +19,8 @@ import { basename, dirname, isAbsolute, join, resolve, sep } from 'node:path';
 
 import { stableJson } from '../core/digest.ts';
 import { isErrno } from '../core/errors.ts';
+import { packageBinEntries } from '../core/package-dependencies.ts';
+import { isRecord } from '../core/strict-json.ts';
 import { matchesManifestFile } from '../build/artifact-layout.ts';
 import { exists, installReceiptFile, isInstallReceiptEntry, isPortablePathSegment, isPreservedRuntimeRoot } from '../core/paths.ts';
 import { stateOwnershipMarkerFile } from '../core/types.ts';
@@ -273,10 +276,11 @@ const hashEntry = (
   relativePath: string,
   metadata: Stats,
   bytes: Uint8Array,
+  executable = (metadata.mode & 0o111) !== 0,
 ): void => {
   hash.update(toPosix(relativePath));
   hash.update('\0');
-  hash.update((metadata.mode & 0o111) === 0 ? '-' : 'x');
+  hash.update(executable ? 'x' : '-');
   hash.update('\0');
   hash.update(bytes);
   hash.update('\0');
@@ -328,14 +332,35 @@ export const treeInventory = async (root: string): Promise<TreeInventory> => {
   return Object.freeze({ files: Object.freeze(files), hash: hash.digest('hex') });
 };
 
+const installedPackageBinTargets = async (root: string): Promise<ReadonlySet<string> | undefined> => {
+  let candidate = resolve(root);
+  while (true) {
+    const parent = dirname(candidate);
+    if (
+      basename(parent) === 'node_modules' ||
+      (basename(parent).startsWith('@') && basename(dirname(parent)) === 'node_modules')
+    ) {
+      const packageDocument: unknown = JSON.parse(await readFile(join(candidate, 'package.json'), 'utf8'));
+      if (!isRecord(packageDocument)) throw new TypeError(`Expected a JSON object at ${JSON.stringify(join(candidate, 'package.json'))}.`);
+      return new Set(packageBinEntries(packageDocument).map(([, target]) => resolve(candidate, target)));
+    }
+    if (parent === candidate) return undefined;
+    candidate = parent;
+  }
+};
+
 /**
  * Reads only the fixed paths declared by the authoritative artifact manifest,
  * plus the manifest itself and conventional operator environment overlays.
  */
 export const manifestInventory = async (
   root: string,
   manifest: ArtifactManifest,
-  options: { readonly verifyHashes?: boolean } = {},
+  options: {
+    readonly hashManifestModes?: boolean;
+    readonly restoreModes?: boolean;
+    readonly verifyHashes?: boolean;
+  } = {},
 ): Promise<TreeInventory> => {
   const rootMetadata = await lstat(root);
   if (rootMetadata.isSymbolicLink() || !rootMetadata.isDirectory()) throw unsupportedEntry('.');
@@ -352,6 +377,7 @@ export const manifestInventory = async (
   }
   const files = [...paths].sort(compareTreePaths);
   await assertRealAncestors(root, files);
+  const npmBinTargets = await installedPackageBinTargets(root);
   const hash = createHash('sha256');
   for (const relativePath of files) {
     const path = join(root, relativePath);
@@ -368,15 +394,44 @@ export const manifestInventory = async (
       throw error;
     }
     const row = rows.get(relativePath);
-    if (options.verifyHashes !== false && row !== undefined && !matchesManifestFile({
+    const file = {
       bytes: metadata.size,
       mode: metadata.mode & 0o777,
       path: relativePath,
       sha256: createHash('sha256').update(bytes).digest('hex'),
-    }, row)) {
+    };
+    // npm forces declared bins executable; other files keep the publisher's executable state while rw bits follow umask.
+    const executable = (file.mode & 0o111) !== 0;
+    const manifestExecutable = row?.mode !== undefined && (row.mode & 0o111) !== 0;
+    const npmModeMatches = npmBinTargets !== undefined && (
+      npmBinTargets.has(resolve(root, relativePath))
+        ? executable
+        : manifestExecutable || !executable
+    );
+    if (
+      options.verifyHashes !== false &&
+      row !== undefined &&
+      !matchesManifestFile(file, row) &&
+      !(npmModeMatches && matchesManifestFile({ ...file, mode: row.mode ?? file.mode }, row))
+    ) {
       throw new Error(`--from root does not match its manifest: ${relativePath} differs from its files[] row in bytes, mode, or digest.`);
     }
-    hashEntry(hash, relativePath, metadata, bytes);
+    if (
+      options.restoreModes === true &&
+      npmModeMatches &&
+      row?.mode !== undefined &&
+      file.mode !== row.mode
+    ) {
+      await chmod(path, row.mode);
+      metadata = await lstat(path);
+    }
+    hashEntry(
+      hash,
+      relativePath,
+      metadata,
+      bytes,
+      options.hashManifestModes === false || row === undefined ? undefined : manifestExecutable,
+    );
   }
   return Object.freeze({ files: Object.freeze(files), hash: hash.digest('hex') });
 };