fix(bundle): read package.json as Node does; remapping errors never quote; green main after #178

doc/file-formats.md

@@ -369,11 +369,15 @@ A `foundry.toml` or `extends` base that isn't TOML, a config that isn't UTF-8
   or `{ path, strategy }`), and an invalid remapping (a `remappings.txt` line or
   `FOUNDRY_REMAPPINGS` entry that isn't `[context:]prefix=target`, or a
   `remappings` value that isn't an array of such strings), is an error naming
-  the file (from the root), whosever it is and in every mode: nothing falls back
+  the file (from the root) and the line or entry, never its text (a file named
+  as a mapping by mistake may hold a secret), whosever it is and in every mode: nothing falls back
   to a default (forge refuses these too, but quietly skips a dependency's
   `foundry.toml` it can't read). So is a config that isn't a regular file: a
   FIFO, a device or a link to one (`remappings.txt -> /dev/stdin`) is never
-  read, so the bundle can't stall on it or take the process's input as config. A `remappings.txt` line is trimmed as forge trims it, so a
+  read, so the bundle can't stall on it or take the process's input as config.
+  So is one there that can't be read, a link loop or a file in a directory that
+  may not be searched: only nothing there (a link to nothing included) is no
+  file, as `stat` tells them apart. A `remappings.txt` line is trimmed as forge trims it, so a
   byte-order mark stays part of the first remapping. A dependency's config forge
   rejects for its settings (a missing `extends` base, nested inheritance) is
   skipped with a warning, as forge skips it.
@@ -455,9 +459,13 @@ path, a dependency outside the root reads nothing, and a dir a dependency's
 `libs` names must be a dependency itself; a config refused says why. A
 `package.json` that decides a file's package is refused the same way when a
 dependency planted it as a link, and one that doesn't parse (a leading
-byte-order mark is skipped, as npm skips it) or isn't a regular file is an error
-naming it (not quoting it) rather than giving its files to the parent package; other bundles walk past
-a malformed one, as they always have. A
+byte-order mark is skipped, as npm skips it), isn't a regular file or can't be
+read (a link loop, a directory that may not be searched) is an error naming it
+(not quoting it) rather than giving its files to the parent package; a link to
+nothing is no `package.json`, as to Node. Other bundles walk past a malformed
+or unreadable one, as they always have, but the JS bundler's own lookups (its
+resolver and its packages' names, on disk and in a Vfs) refuse one they can't
+read as Node does (`ERR_INVALID_PACKAGE_CONFIG`). A
 link the project placed (a workspace package linked into `node_modules`, a
 linked `lib/` entry, `src/vendor`) may lead anywhere in the root, and so may one
 on the path the project was named by (a symlinked checkout); a workspace package

stasis-core/src/bundle-util.js

@@ -44,36 +44,61 @@ export function findPackageMetadata(baseDir, fileRelPath, { strict = false, chec
 // The error codes that mean nothing is at a path.
 export const NO_ENTRY = new Set(['ENOENT', 'ENOTDIR'])
 
+// Why `file` can't be read, where `host.stat` gave null, or null when nothing is there (a missing
+// path, one through a file, a link to nothing). host.stat answers null for any failure, as Node's
+// module lookup does; real stat tells them apart: a link loop, a directory that may not be searched
+// or a name too long is something there that can't be read, and a read meets the same error.
+function readFailure(host, file) {
+  try {
+    host.readFile(file)
+  } catch (err) {
+    return NO_ENTRY.has(err.code) ? null : err
+  }
+  return new Error(`${file} could be read but not stat'ed`)
+}
+
+// host.stat as real stat answers: null only when nothing is there (readFailure); anything else that
+// can't be stat'ed throws, naming the file `label`.
+export function statStrict(host, file, label) {
+  const stat = host.stat(file)
+  if (stat !== null) return stat
+  const failure = readFailure(host, file)
+  if (failure === null) return null
+  throw new Error(`${label}: can't be read (${failure.code ?? failure.message})`, { cause: failure })
+}
+
+// host.stat for a package.json, as Node's lookups read one: null when nothing is there, and one that
+// can't be read (a link loop, a directory that may not be searched) throws ERR_INVALID_PACKAGE_CONFIG,
+// as Node refuses it.
+export function packageJSONStat(host, file) {
+  const stat = host.stat(file)
+  if (stat !== null) return stat
+  const cause = readFailure(host, file)
+  if (cause === null) return null
+  throw Object.assign(new Error(`Cannot read package config ${file}: ${cause.code ?? cause.message}.`, { cause }), { code: 'ERR_INVALID_PACKAGE_CONFIG' })
+}
+
 // `file`'s bytes, read through `host`, or null when there's no file (a directory counts as none).
 // It's read only when it's a regular file: a FIFO, a socket, a device or a link to one
-// (`/dev/stdin`) throws, naming it `label`, rather than stalling or reading the process's input.
-// What can't be stat'ed is read to say why: only a path with nothing there is no file, and a loop
-// or a directory that may not be searched throws.
+// (`/dev/stdin`) throws, naming it `label`, rather than stalling or reading the process's input, and
+// so does one there that can't be read (statStrict).
 export function readRegularFileOrNull(file, label, host = diskHost) {
-  const stat = host.stat(file)
-  if (stat === null) {
-    try {
-      host.readFile(file)
-    } catch (err) {
-      if (NO_ENTRY.has(err.code)) return null
-      throw err
-    }
-    throw new Error(`${label}: not a regular file`)
-  }
-  if (stat.isDirectory()) return null
+  const stat = statStrict(host, file, label)
+  if (stat === null || stat.isDirectory()) return null
   if (!stat.isFile()) throw new Error(`${label}: not a regular file`)
   return host.readFile(file)
 }
 
 // The package.json at `rel` (under `baseDir`), parsed (a leading byte-order mark skipped, as npm
 // and Node skip it), read through `host`; null when there's none (a directory counts as none), or
-// when it doesn't parse or isn't a regular file -- unless `strict`, then that throws, saying where
+// when it doesn't parse, isn't a regular file or can't be read -- unless `strict`, then that throws
+// (only nothing there is no package.json, as Node reads one: statStrict), saying where
 // with the parser's line and column but never its message, which quotes the text (a file that isn't
 // JSON may be anything, a secret included). `check(rel)`, when given, sees the path before it is
 // read, and may throw to refuse it.
 export function readPackageJson(baseDir, rel, { strict = false, check, host = diskHost } = {}) {
   const file = join(baseDir, rel)
-  const stat = host.stat(file)
+  const stat = strict ? statStrict(host, file, rel) : host.stat(file)
   if (stat === null || stat.isDirectory()) return null
   check?.(rel)
   try {
@@ -113,10 +138,11 @@ export function normalizeEntries(entries, cwd) {
   })
 }
 
-// Bytes of a bundled module's `package.json`, or null to skip when it's absent; non-UTF-8 aborts (never silently skipped).
+// Bytes of a bundled module's `package.json`, or null to skip when it's absent; one that can't be
+// read (statStrict) or isn't UTF-8 aborts (never silently skipped).
 export function readModuleManifest({ baseDir, realBase, rel, host = diskHost } = {}) {
   const absolute = join(baseDir, rel)
-  if (host.stat(absolute) === null) return null
+  if (statStrict(host, absolute, rel) === null) return null
   assertRealPathWithinBase(realBase, baseDir, rel, host)
   const buf = host.readFile(absolute)
   if (!isUtf8(buf)) throw new Error(`package.json is not valid UTF-8: ${rel}`)

stasis-core/src/state.js

@@ -12,7 +12,7 @@ import { parseShard, serializeShard } from './shard.js'
 import { canonicalizePath, sha512integrity, readFileSyncMaybe, noupsert } from './state-util.js'
 import { brotliOptions } from './brotli.js'
 import { CODE_EXTENSIONS, canObserveExecuteBits, classifyFormat, erasedTypeScriptFormat, fileMapToObject, hasNodeModulesSegment, isBinaryPlist, isNativeArtifact, isStatFormat, moduleFileKey, narrowExecutable, objectToMaps, observeExecutable, pathExt, reconcileFormat, sortPaths, splitNodeModulesPath } from './util.js'
-import { detectRepo, packageJSONText, readModuleManifest } from './bundle-util.js'
+import { detectRepo, packageJSONStat, packageJSONText, readModuleManifest } from './bundle-util.js'
 import { diskHost } from './host.js'
 import corePackage from './package.cjs'
 
@@ -254,7 +254,8 @@ export class State {
     const potentialRoots = []
     let cursor = root
     while (cursor) {
-      if (this.#exists(join(cursor, 'package.json'))) {
+      // One there that can't be read is refused, not walked past to a root above (packageJSONStat).
+      if (packageJSONStat(this.#host, join(cursor, 'package.json')) !== null) {
         potentialRoots.push(cursor)
       } else if (
         this.#exists(join(cursor, FILE_CONFIG)) ||
@@ -731,12 +732,13 @@ export class State {
   }
 
   // Nearest package.json at or above a directory (findPackageJSON is unreliable for a directory
-  // URL, see #locateModule). Bounded by the project root.
+  // URL, see #locateModule), refusing one there that can't be read as findPackageJSON does
+  // (packageJSONStat). Bounded by the project root.
   #nearestPackageJsonFor(dirAbsolute) {
     let dir = dirAbsolute
     while (true) {
       const candidate = join(dir, 'package.json')
-      if (this.#host.stat(candidate)?.isFile()) return candidate
+      if (packageJSONStat(this.#host, candidate)?.isFile()) return candidate
       if (dir === this.root) break // checked the root's package.json; never escape root
       const parent = dirname(dir)
       if (parent === dir) break

stasis/src/cmd/bundle.js

@@ -285,10 +285,12 @@ export const isSolidityEntry = (entry, cwd = process.cwd(), host = diskHost) =>
 // What's wrong with `entries`' directory entries (resolved against `cwd`), or null: a directory
 // entry stands for the .sol files under it, so it goes with Solidity entries only; and entries
 // that are all missing extensionless paths are a mistyped file, not a project without those dirs.
-export function directoryEntryError(entries, cwd = process.cwd(), host = diskHost) {
+// `fetched` false: `host` is an empty tree standing in for one not fetched yet, which can't say
+// what is missing.
+export function directoryEntryError(entries, cwd = process.cwd(), host = diskHost, { fetched = true } = {}) {
   const dirs = entries.filter((e) => isDirEntry(resolve(cwd, e), host))
-  const absent = dirs.filter((e) => host.stat(resolve(cwd, e)) === null)
-  if (absent.length === entries.length) return `no such file or directory: ${absent[0]}`
+  const absent = fetched ? dirs.filter((e) => host.stat(resolve(cwd, e)) === null) : []
+  if (absent.length > 0 && absent.length === entries.length) return `no such file or directory: ${absent[0]}`
   if (dirs.length === 0 || entries.every((e) => e.endsWith('.sol') || dirs.includes(e))) return null
   return absent.length > 0
     ? `no such file or directory: ${absent[0]}`
@@ -1035,11 +1037,11 @@ async function buildResolvedJsBundle({ cwd = process.cwd(), entries, mainFields,
 
 // Classify entries into their single shared language and check option applicability; `name` prefixes
 // errors. A directory entry (resolved against `cwd`, on `host`) stands for the .sol files under it: Solidity only.
-function classifyEntries(name, { cwd = process.cwd(), entries, mappingFile, manifests, scope, lockfile, conditions, mainFields, platforms, metro, metroResolver, jsx, flow, typescript, tsconfig, resources, packageJSON, cargo, cargoFeatures, cargoNoDefaultFeatures, cargoAllFeatures, host = diskHost }) {
+function classifyEntries(name, { cwd = process.cwd(), entries, mappingFile, manifests, scope, lockfile, conditions, mainFields, platforms, metro, metroResolver, jsx, flow, typescript, tsconfig, resources, packageJSON, cargo, cargoFeatures, cargoNoDefaultFeatures, cargoAllFeatures, host = diskHost, fetched }) {
   if (!Array.isArray(entries) || entries.length === 0) {
     throw new Error(`${name}: at least one entry file is required`)
   }
-  const dirError = directoryEntryError(entries, cwd, host)
+  const dirError = directoryEntryError(entries, cwd, host, { fetched })
   if (dirError !== null) throw new Error(`${name}: ${dirError}`)
   let kind
   if (entries.every((e) => isSolidityEntry(e, cwd, host))) kind = 'sol'
@@ -1161,7 +1163,8 @@ async function buildJs({ mainFields, platforms, metro, metroResolver, ...options
 }
 
 // buildVfsBundle's options for the package manager `pm`, checked before anything is fetched (`host`
-// the project's): its kind alone, and no metro-resolver, which reads the disk.
+// the project's, or an empty tree's with `fetched: false`, before the project is fetched): its kind
+// alone, and no metro-resolver, which reads the disk.
 export function checkVfsOptions(name, pm, packageManager, options) {
   if (classifyEntries(name, options) !== pm.kind) throw new Error(`${name}: only ${pm.kind === 'sol' ? 'Solidity' : 'JS'} bundles are built with ${packageManager}`)
   if (options.metroResolver) throw new Error(`${name}: metroResolver is not supported`)

stasis/src/loaders/foundry.js

@@ -134,6 +134,10 @@ export function parseRemapping(entry, { emptyPath = false } = {}) {
   return { context, name, path }
 }
 
+// What an invalid remapping should have been; errors name where one is, never its text, which may
+// be anything (a file named as a mapping by mistake, a secret included).
+const REMAPPING_FORM = 'expected [context:]prefix=target'
+
 // A remappings.txt / env var body: one remapping per non-blank (trimmed) line. A line that isn't
 // one throws, naming `label` (the file or variable) and the line, as forge and solc refuse the
 // file. `emptyPath`: see parseRemapping.
@@ -143,20 +147,21 @@ export function parseRemappingLines(text, { label = 'remappings', emptyPath = fa
     const line = rustTrim(raw)
     if (line === '') return
     const r = parseRemapping(line, { emptyPath })
-    if (r === null) throw new Error(`${label}:${i + 1}: invalid remapping ${JSON.stringify(line)}`)
+    if (r === null) throw new Error(`${label}:${i + 1}: invalid remapping, ${REMAPPING_FORM}`)
     out.push(r)
   })
   return out
 }
 
 // A foundry.toml's `remappings` value, parsed. One forge rejects -- not an array of strings, or an
-// entry that isn't `[context:]name=path` -- throws, naming `file` when given.
+// entry that isn't `[context:]name=path` -- throws, naming `file` when given and the entry (from 1).
 function configRemappings(value, file) {
   const where = `${file === null ? '' : `${file}: `}\`remappings\``
   if (!Array.isArray(value)) throw new Error(`${where} is not an array of strings`)
-  return value.map((entry) => {
-    const r = typeof entry === 'string' ? parseRemapping(entry) : null
-    if (r === null) throw new Error(`${where}: invalid remapping ${typeof entry === 'string' ? JSON.stringify(entry) : String(entry)}`)
+  return value.map((entry, i) => {
+    if (typeof entry !== 'string') throw new Error(`${where} entry ${i + 1} is not a string`)
+    const r = parseRemapping(entry)
+    if (r === null) throw new Error(`${where} entry ${i + 1}: invalid remapping, ${REMAPPING_FORM}`)
     return r
   })
 }

stasis/src/resolve-node.js

@@ -2,7 +2,7 @@ import Module, { isBuiltin } from 'node:module'
 import { basename, dirname, isAbsolute, join, normalize, resolve } from 'node:path'
 import { fileURLToPath, pathToFileURL } from 'node:url'
 
-import { packageJSONText } from '@exodus/stasis-core/bundle-util'
+import { packageJSONStat, packageJSONText } from '@exodus/stasis-core/bundle-util'
 
 // Node's CommonJS resolution (`createRequire(parent).resolve(spec, { conditions })`) over a
 // `host` (@exodus/stasis-core/host), mirroring lib/internal/modules/cjs/loader.js and
@@ -88,9 +88,16 @@ function parsePackageName(specifier, base) {
 }
 
 export function createNodeResolver(host) {
-  // package.json reads, memoized per path; a malformed manifest throws ERR_INVALID_PACKAGE_CONFIG on every access.
+  // package.json reads, memoized per path; a malformed manifest, or one there that can't be read,
+  // throws ERR_INVALID_PACKAGE_CONFIG on every access.
   const parsePackage = (pjsonPath) => {
-    if (!host.stat(pjsonPath)?.isFile()) return { exists: false }
+    let stat
+    try {
+      stat = packageJSONStat(host, pjsonPath)
+    } catch (err) {
+      return err
+    }
+    if (!stat?.isFile()) return { exists: false }
     let data
     try {
       data = JSON.parse(packageJSONText(host.readFile(pjsonPath)))

stasis/src/vfs-bundle.js

@@ -60,7 +60,7 @@ export async function suggestedEntries({ vfs, cwd = '/', conditions, mainFields,
   if (vfs === undefined && repo.github === undefined) throw new Error('suggestedEntries: a vfs or a github repo is required')
   if (vfs !== undefined && repo.github !== undefined) throw new Error('suggestedEntries: takes a vfs or a github repo, not both')
   // Over a JS entry, as each suggested one is.
-  checkVfsOptions('suggestedEntries', { kind: 'js' }, undefined, { ...resolution, entries: ['index.js'], cwd: '/', host: vfsHost(new Vfs()) })
+  checkVfsOptions('suggestedEntries', { kind: 'js' }, undefined, { ...resolution, entries: ['index.js'], cwd: '/', host: vfsHost(new Vfs()), fetched: false })
   if (vfs === undefined) return suggestedRepoEntries({ ...repo, ...resolution })
   checkVfs('suggestedEntries', vfs)
   return packageEntries(vfsHost(vfs), posix.resolve('/', cwd), resolution)

stasis/src/vfs-bundle/github.js

@@ -108,7 +108,7 @@ export async function buildGitHubBundle({ github, sha, directory, client, packag
   if (options.entries === undefined && pm.kind !== 'js') throw new Error(`buildGitHubBundle: entries are required with ${packageManager}`)
   // buildVfsBundle's checks, over an empty tree: what the tree is never decides them. Without
   // entries, over a JS one, as each suggested one is.
-  checkVfsOptions('buildGitHubBundle', pm, packageManager, { ...options, entries: options.entries ?? ['index.js'], cwd: '/', host: vfsHost(new Vfs()) })
+  checkVfsOptions('buildGitHubBundle', pm, packageManager, { ...options, entries: options.entries ?? ['index.js'], cwd: '/', host: vfsHost(new Vfs()), fetched: false })
   checkRepo('buildGitHubBundle', { github, sha, directory })
   client ??= createClient({ token: null })
   sha ??= (await client.getRepoHead({ repo: github })).oid

stasis/src/vfs-bundle/tree.js

@@ -1,7 +1,7 @@
 import { constants } from 'node:fs'
 import { basename, dirname, join, relative, resolve, sep } from 'node:path'
 
-import { packageJSONText, readJson } from '@exodus/stasis-core/bundle-util'
+import { packageJSONStat, packageJSONText, readJson } from '@exodus/stasis-core/bundle-util'
 import { byName } from '@exodus/stasis-core/host'
 import { hasNodeModulesSegment } from '@exodus/stasis-core/util'
 import { buildPnpmTree, findPnpmProjects } from '@preventive/deptree/pnpm.js'
@@ -442,13 +442,14 @@ export function vfsHost(vfs, { root, outside, installs = [], hides, cache = true
     realpath(p) {
       return realpath(abs(p))
     },
-    // As Node's: the nearest package.json above a file's real path, never out of a node_modules dir.
+    // As Node's: the nearest package.json above a file's real path, never out of a node_modules dir;
+    // one there that can't be read is refused (packageJSONStat).
     findPackageJSON(p) {
       let from = abs(p)
       if (host.stat(from)?.isFile()) from = realpath(from)
       for (let dir = dirname(from); basename(dir) !== 'node_modules'; dir = dirname(dir)) {
         const candidate = join(dir, 'package.json')
-        if (host.stat(candidate)?.isFile()) {
+        if (packageJSONStat(host, candidate)?.isFile()) {
           checkManifest(candidate)
           return candidate
         }