fix(build): parameterize the symlink policy so verify-pins survives no-install (24)

assertSafeSymlinks unconditionally realpathSync'd the workspace node_modules
root, so the advisory verify-binary-pins.yml job — which deliberately skips
`pnpm install` — threw ENOENT on a clean checkout and failed PR #19's
verify-pins check.

Replace the implicit global allowlist with an explicit policy every call site
must state: OWN_ROOT_ONLY (foreign tarball/cache trees and the digest
generator's scratch extraction — only the tree's own canonical root may be a
link target) vs ALLOW_WORKSPACE_NODE_MODULES (installed pnpm packages, where
the workspace node_modules root is additionally a legitimate target). A
missing node_modules under ALLOW_WORKSPACE_NODE_MODULES is now tolerated —
absence just means that allowance doesn't apply, not a crash.

Wire the correct policy into all four call sites (vendor-platform-packages.mjs
x2, prepare-package-deps.mjs, record-native-binary-digests.mjs) and add
scripts/safe-symlinks.test.mjs (node:test, 12 cases) covering both policies,
the missing-node_modules case, and genuine escape attempts under each policy
that must still be refused. Wired into `pnpm test` via a new `test:scripts`
script.

Also harden verify-binary-pins.yml: add `permissions: contents: read`, and
extend its path triggers to scripts/safe-symlinks.mjs,
scripts/vendor-platform-packages.mjs, and package.json so a future change to
the check itself re-runs the job. The advisory-not-gate framing in the
workflow's header comment is untouched.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Author: ARHAEEM

.github/workflows/verify-binary-pins.yml

@@ -64,6 +64,12 @@ on:
       - 'scripts/native-binary-digests.json'      # the pins themselves
       - 'scripts/record-native-binary-digests.mjs' # HOW digests are produced
       - 'scripts/vsix-targets.mjs'                # WHICH packages are expected at all
+      - 'scripts/safe-symlinks.mjs'                # the symlink-escape guard the check relies on
+      - 'scripts/vendor-platform-packages.mjs'     # tarball fetch/verify/vendor logic the check imports
+      - 'package.json'                             # the `check:binary-digests` script definition itself
+
+permissions:
+  contents: read
 
 jobs:
   verify-pins:

package.json

@@ -11,7 +11,8 @@
     "check:binary-digests": "node scripts/record-native-binary-digests.mjs --check",
     "check:tool-sync": "node scripts/check-tool-sync.mjs",
     "check:bundle-sync": "node scripts/check-bundle-sync.mjs",
-    "test": "node scripts/check-tool-sync.mjs && pnpm -F shared test && pnpm -F language-services test && pnpm -F airtable-user-mcp test && pnpm -F webview test && pnpm -F airtable-formula test",
+    "test:scripts": "node --test scripts/safe-symlinks.test.mjs",
+    "test": "node scripts/check-tool-sync.mjs && pnpm test:scripts && pnpm -F shared test && pnpm -F language-services test && pnpm -F airtable-user-mcp test && pnpm -F webview test && pnpm -F airtable-formula test",
     "packx": "pnpm -F airtable-formula packx",
     "packx:no-bump": "pnpm -F airtable-formula packx:no-bump",
     "version:bump": "pnpm -F airtable-formula version:bump",

scripts/prepare-package-deps.mjs

@@ -25,7 +25,7 @@ import { fileURLToPath } from 'node:url';
 import { createRequire } from 'node:module';
 import { ALL_TARGETS, hostTarget, isPlatformPackage, targetConfig } from './vsix-targets.mjs';
 import { vendorPlatformPackagesForTarget } from './vendor-platform-packages.mjs';
-import { assertSafeSymlinks } from './safe-symlinks.mjs';
+import { assertSafeSymlinks, SYMLINK_POLICY } from './safe-symlinks.mjs';
 
 const __dirname = dirname(fileURLToPath(import.meta.url));
 const require = createRequire(import.meta.url);
@@ -98,8 +98,11 @@ function resolvePackageRoot(packageName) {
 }
 
 // The symlink-escape guard `copyPackage` applies before every dereferencing
-// copy now lives in `safe-symlinks.mjs`, so the foreign-platform vendoring path
-// (`vendor-platform-packages.mjs`) applies the identical check.
+// copy now lives in `safe-symlinks.mjs`, shared with the foreign-platform
+// vendoring path (`vendor-platform-packages.mjs`) — though the two apply
+// different policies: this path resolves an installed pnpm package (whose own
+// links legitimately point into the workspace node_modules .pnpm store),
+// while the foreign-tarball path allows only that tree's own root.
 
 /**
  * Resolve + safety-check + copy a single package into dist/node_modules.
@@ -117,7 +120,9 @@ function copyPackage(packageName) {
     return false;
   }
 
-  assertSafeSymlinks(realSource);
+  // Resolved from an installed pnpm package: its own dependency links
+  // legitimately resolve into the workspace node_modules .pnpm store.
+  assertSafeSymlinks(realSource, SYMLINK_POLICY.ALLOW_WORKSPACE_NODE_MODULES);
 
   const target = join(extensionNodeModules, packageName);
   // dereference: true — follow symlinks and copy real files, required for

scripts/record-native-binary-digests.mjs

@@ -56,7 +56,7 @@ import {
   findNodeBinaries,
   sha256Hex,
 } from './vendor-platform-packages.mjs';
-import { assertSafeSymlinks } from './safe-symlinks.mjs';
+import { assertSafeSymlinks, SYMLINK_POLICY } from './safe-symlinks.mjs';
 
 const __dirname = dirname(fileURLToPath(import.meta.url));
 const OUT_FILE = join(__dirname, 'native-binary-digests.json');
@@ -85,8 +85,12 @@ async function digestsFor(name, scratch) {
   // hash THAT, pinning a digest of something we never unpacked. Unreachable
   // today (the tarball is byte-identical to what the registry published, per the
   // integrity gate above), but this is the routine that mints the trust root for
-  // every later check, so it should not rely on an upstream guarantee.
-  assertSafeSymlinks(unpacked);
+  // every later check, so it should not rely on an upstream guarantee. This is
+  // a throwaway scratch extraction, not an installed package, so only its own
+  // root is an allowed link target — and, notably, this call never touches
+  // node_modules at all, which is what lets `--check` run in the advisory
+  // verify-pins CI job that deliberately skips `pnpm install`.
+  assertSafeSymlinks(unpacked, SYMLINK_POLICY.OWN_ROOT_ONLY);
 
   const manifest = JSON.parse(readFileSync(join(unpacked, 'package.json'), 'utf8'));
   if (manifest.name !== name || manifest.version !== version) {

scripts/safe-symlinks.mjs

@@ -8,24 +8,42 @@
  * inside the published artifact.
  *
  * Before copying, walk the tree (following directory symlinks, cycle-safe) and
- * require every symlink to resolve inside an allowed root. pnpm's legitimate
- * nested-dependency links all resolve into <workspace>/node_modules/.pnpm, so
- * that path plus the tree's own root is the default allowlist.
+ * require every symlink to resolve inside an allowed root. There are two
+ * legitimately different sets of allowed roots, so the caller must say which
+ * one applies — there is no default, because silently picking one back into a
+ * single implicit global is exactly the mistake this replaces:
+ *
+ *   OWN_ROOT_ONLY — for a foreign tarball/cache tree (registry tarballs
+ *     unpacked by `tar`, which happily creates whatever symlinks the archive
+ *     asks for) or the digest generator's own throwaway scratch extraction.
+ *     Only the tree's own canonical root may be a link target; nothing
+ *     legitimate ever needs to point further out.
+ *
+ *   ALLOW_WORKSPACE_NODE_MODULES — for a tree that IS (or is copied from) an
+ *     installed pnpm package. pnpm's store layout legitimately nests real
+ *     dependency links that resolve into the workspace `node_modules` root
+ *     (the `.pnpm` virtual store), so that root is additionally allowed.
  *
  * Lives here rather than in `prepare-package-deps.mjs` so the foreign-package
  * path (`vendor-platform-packages.mjs`, which unpacks registry tarballs with
- * `tar` — and `tar` happily creates symlinks) gets the identical guard the
- * non-foreign path already had.
+ * `tar`) gets the identical walking/comparison logic the installed-package
+ * path already had — only the allowed-roots policy differs between them.
  */
 
-import { lstatSync, readdirSync, realpathSync } from 'node:fs';
+import { existsSync, lstatSync, readdirSync, realpathSync } from 'node:fs';
 import { dirname, join, sep } from 'node:path';
 import { fileURLToPath } from 'node:url';
 
 const __dirname = dirname(fileURLToPath(import.meta.url));
 
 const workspaceRoot = realpathSync(join(__dirname, '..'));
 
+/** The two symlink policies a call site can choose. See file header. */
+export const SYMLINK_POLICY = Object.freeze({
+  OWN_ROOT_ONLY: 'own-root-only',
+  ALLOW_WORKSPACE_NODE_MODULES: 'allow-workspace-node-modules',
+});
+
 // Windows paths are case-insensitive; normalize before prefix comparison.
 const normalizeForCompare = (p) => (process.platform === 'win32' ? p.toLowerCase() : p);
 
@@ -36,14 +54,46 @@ const isWithin = (child, parent) => {
 };
 
 /**
- * Throw if any symlink under `rootDir` resolves outside `rootDir` or the
- * workspace `node_modules` tree, or is broken.
+ * Throw if any symlink under `rootDir` resolves outside the roots `policy`
+ * allows, or is broken.
+ *
+ * `policy` is required — one of `SYMLINK_POLICY.OWN_ROOT_ONLY` or
+ * `SYMLINK_POLICY.ALLOW_WORKSPACE_NODE_MODULES`; see the file header for which
+ * one a given call site needs.
+ *
+ * `workspaceNodeModulesRoot` is a test-only override for the workspace
+ * `node_modules` path `ALLOW_WORKSPACE_NODE_MODULES` consults; production call
+ * sites never pass it and get the real workspace root.
  */
-export function assertSafeSymlinks(rootDir) {
-  // Both roots must be canonicalized — symlink targets are compared after
-  // realpathSync, so an un-resolved allowlist entry (workspace itself behind
-  // a symlink, e.g. a git worktree) would reject legitimate pnpm links.
-  const allowedRoots = [realpathSync(rootDir), realpathSync(join(workspaceRoot, 'node_modules'))];
+export function assertSafeSymlinks(
+  rootDir,
+  policy,
+  { workspaceNodeModulesRoot = join(workspaceRoot, 'node_modules') } = {}
+) {
+  if (policy !== SYMLINK_POLICY.OWN_ROOT_ONLY && policy !== SYMLINK_POLICY.ALLOW_WORKSPACE_NODE_MODULES) {
+    throw new Error(
+      'assertSafeSymlinks: a policy is required — pass SYMLINK_POLICY.OWN_ROOT_ONLY or ' +
+      `SYMLINK_POLICY.ALLOW_WORKSPACE_NODE_MODULES (got: ${JSON.stringify(policy)})`
+    );
+  }
+
+  // The root must be canonicalized — symlink targets are compared after
+  // realpathSync, so an un-resolved allowlist entry (e.g. the workspace
+  // itself behind a symlink, as in a git worktree) would reject legitimate
+  // pnpm links.
+  const allowedRoots = [realpathSync(rootDir)];
+
+  if (policy === SYMLINK_POLICY.ALLOW_WORKSPACE_NODE_MODULES) {
+    // A no-install checkout (e.g. the advisory verify-pins CI job, which
+    // deliberately skips `pnpm install` because it needs only Node builtins,
+    // the repo's scripts, the lockfile and `tar`) has no node_modules at all.
+    // Absence is not a violation — it just means this allowance does not
+    // apply, and the tree is held to its own root only.
+    if (existsSync(workspaceNodeModulesRoot)) {
+      allowedRoots.push(realpathSync(workspaceNodeModulesRoot));
+    }
+  }
+
   const visited = new Set();
   const stack = [realpathSync(rootDir)];
 
@@ -70,7 +120,7 @@ export function assertSafeSymlinks(rootDir) {
         }
         if (!allowedRoots.some((root) => isWithin(resolved, root))) {
           throw new Error(
-            `Refusing to package: symlink escapes workspace node_modules:\n  ${entryPath}\n  → ${resolved}`
+            `Refusing to package: symlink escapes its allowed root:\n  ${entryPath}\n  → ${resolved}`
           );
         }
         // dereference:true copies the target's contents too — keep walking