From 63a73caa78ed99f9a3b83bbabf7a7030fd4fcdc1 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 08:59:40 +0000 Subject: [PATCH 1/3] fix(vfs-bundle): check entries before the fetch without looking for them buildGitHubBundle and suggestedEntries check their options against an empty Vfs, before anything is fetched: "what the tree is never decides them" (#197). #178's directoryEntryError then found every extensionless entry missing from that empty tree. So `src` with pnpm was refused as "no such file or directory" rather than "only JS bundles are built with pnpm", and main's "buildGitHubBundle reads nothing from disk" test fails. The pre-fetch checks now pass `fetched: false`, which skips the missing-entry errors. buildVfsBundle still checks entries against the project's own tree. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01C6oBS5QX4oqZcd2d3STiGA --- stasis/src/cmd/bundle.js | 15 +++++++++------ stasis/src/vfs-bundle.js | 2 +- stasis/src/vfs-bundle/github.js | 2 +- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/stasis/src/cmd/bundle.js b/stasis/src/cmd/bundle.js index 59d42a56..7b6a02b6 100644 --- a/stasis/src/cmd/bundle.js +++ b/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`) diff --git a/stasis/src/vfs-bundle.js b/stasis/src/vfs-bundle.js index cee83b23..35ce9c15 100644 --- a/stasis/src/vfs-bundle.js +++ b/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) diff --git a/stasis/src/vfs-bundle/github.js b/stasis/src/vfs-bundle/github.js index c8a49d59..31be7578 100644 --- a/stasis/src/vfs-bundle/github.js +++ b/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 From 75490a5e37ded4802a2bce42821a60b372021023 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 08:59:40 +0000 Subject: [PATCH 2/3] fix(bundle): a package.json that's there but can't be read is refused, as Node refuses it host.stat returns null for any failure, as Node's module lookup does. So every package.json reader took a link loop, or a file behind a directory that may not be searched, for no package.json and walked past it: - the strict Solidity reader, which bucketed a forge lib's files as the project's own; - the resolver the Vfs host uses, and the Vfs host's findPackageJSON; - State's walk up to a bucket's name, and its root discovery; - readModuleManifest. Node refuses such a package.json with ERR_INVALID_PACKAGE_CONFIG, and counts only "nothing there" as none, a link to nothing included. Real stat tells the two apart and host.stat doesn't. That's right for the module and lib probes, which treat any failure as absent as Node and Rust do, so stat stays as it is. When a package.json reader's stat fails, a read now says why: - statStrict, for configs and the strict package.json read, names the file from the project root (`lib/dep/package.json: can't be read (ELOOP)`). readRegularFileOrNull uses it too, instead of rethrowing the raw fs error with its absolute path. - packageJSONStat throws Node's ERR_INVALID_PACKAGE_CONFIG, for the resolver, the Vfs host's findPackageJSON and State. Lenient lookups (Bash, Rust, `stasis add`, --package-json) still walk past a malformed or unreadable one. A differential against Node's require.resolve and findPackageJSON now agrees on every case: missing, dangling, a directory, malformed, a self-loop, a loop through directories, and (unprivileged) a link into an unsearchable directory. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01C6oBS5QX4oqZcd2d3STiGA --- doc/file-formats.md | 15 +++++--- stasis-core/src/bundle-util.js | 62 ++++++++++++++++++++++++---------- stasis-core/src/state.js | 10 +++--- stasis/src/resolve-node.js | 13 +++++-- stasis/src/vfs-bundle/tree.js | 7 ++-- tests/bundle-cmd.test.js | 27 +++++++++++++++ tests/vfs-bundle-host.test.js | 39 +++++++++++++++++++++ 7 files changed, 141 insertions(+), 32 deletions(-) diff --git a/doc/file-formats.md b/doc/file-formats.md index 67a3ff73..5de2b6c8 100644 --- a/doc/file-formats.md +++ b/doc/file-formats.md @@ -373,7 +373,10 @@ A `foundry.toml` or `extends` base that isn't TOML, a config that isn't UTF-8 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 +458,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 diff --git a/stasis-core/src/bundle-util.js b/stasis-core/src/bundle-util.js index 4164193d..1a03dd38 100644 --- a/stasis-core/src/bundle-util.js +++ b/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}`) diff --git a/stasis-core/src/state.js b/stasis-core/src/state.js index 06390aa1..f5368331 100644 --- a/stasis-core/src/state.js +++ b/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 diff --git a/stasis/src/resolve-node.js b/stasis/src/resolve-node.js index e0cfc7c0..72caa3fa 100644 --- a/stasis/src/resolve-node.js +++ b/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))) diff --git a/stasis/src/vfs-bundle/tree.js b/stasis/src/vfs-bundle/tree.js index f8dfa4f0..3abf20b4 100644 --- a/stasis/src/vfs-bundle/tree.js +++ b/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 } diff --git a/tests/bundle-cmd.test.js b/tests/bundle-cmd.test.js index cde86e10..8ccbff7b 100644 --- a/tests/bundle-cmd.test.js +++ b/tests/bundle-cmd.test.js @@ -1050,6 +1050,33 @@ test('buildSolidityBundle refuses a .sol file that isn\'t UTF-8, rather than bun t.assert.equal(bundle.sources.get('src/B.sol'), '\uFEFFcontract B {}\n') })) +test('buildSolidityBundle fails on a package.json or config that is there but can\'t be read; one that leads nowhere is none, as Node reads it', withTmp(async (t, tmp) => { + writeProject(tmp, { 'foundry.toml': '[profile.default]\n', 'package.json': '{"name":"proj","version":"1.0.0"}', 'src/A.sol': 'import "dep/D.sol";\n', 'lib/dep/src/D.sol': 'contract D {}\n' }) + const build = () => buildSolidityBundle({ cwd: tmp, entries: ['src'], env: {} }) + // A dependency's package.json that loops: walked past, the dependency's files would be bucketed + // as the project's. + symlinkSync('package.json', join(tmp, 'lib/dep/package.json')) + await t.assert.rejects(build, { message: "lib/dep/package.json: can't be read (ELOOP)" }) + // A link to nothing is no package.json, as to Node. + rmSync(join(tmp, 'lib/dep/package.json')) + symlinkSync('gone.json', join(tmp, 'lib/dep/package.json')) + t.assert.deepEqual([...(await build()).sources.keys()].toSorted(), ['lib/dep/src/D.sol', 'src/A.sol']) + // The project's own config that loops: named from the root. + symlinkSync('remappings.txt', join(tmp, 'remappings.txt')) + await t.assert.rejects(build, { message: "remappings.txt: can't be read (ELOOP)" }) +})) + +test('buildBundle refuses a package.json it can\'t read on the walk up to a bucket\'s name, as it refuses a malformed one', withTmp(async (t, tmp) => { + // pkg/sub/package.json marks a type only: the bucket's name is pkg/package.json's. + writeProject(tmp, { '.git/HEAD': '', 'package.json': '{"name":"proj","version":"1.0.0"}', 'index.cjs': "require('./pkg/sub/a.js')\n", 'pkg/sub/package.json': '{"type":"commonjs"}', 'pkg/sub/a.js': '' }) + const build = () => buildBundle({ cwd: tmp, entries: ['index.cjs'] }) + writeFileSync(join(tmp, 'pkg/package.json'), '{ bad') + await t.assert.rejects(build, { code: 'ERR_INVALID_PACKAGE_CONFIG' }) + rmSync(join(tmp, 'pkg/package.json')) + symlinkSync('package.json', join(tmp, 'pkg/package.json')) + await t.assert.rejects(build, { code: 'ERR_INVALID_PACKAGE_CONFIG' }) +})) + test('buildSolidityBundle never stalls on a package.json that isn\'t a regular file', withTmp(async (t, tmp) => { writeProject(tmp, { 'contracts/A.sol': 'import "pkg/P.sol";\n', 'node_modules/pkg/P.sol': 'contract P {}\n' }) // A FIFO: read blocking, it would wait for a writer forever. diff --git a/tests/vfs-bundle-host.test.js b/tests/vfs-bundle-host.test.js index 8f1262e8..2dfaf1b8 100644 --- a/tests/vfs-bundle-host.test.js +++ b/tests/vfs-bundle-host.test.js @@ -1,5 +1,6 @@ import { test } from 'node:test' import { mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from 'node:fs' +import { createRequire } from 'node:module' import { tmpdir } from 'node:os' import { join } from 'node:path' @@ -206,6 +207,44 @@ test('a tsconfig extends a base in node_modules through the tree, never the one t.assert.deepEqual(loadTsconfigPaths('/p/tsconfig.json', createVfsHost(vfs)).matchPaths('@installed/a'), ['/p/node_modules/base/installed/a'], 'the project\'s Vfs alone holds the installed one') }) +test('a package.json that is there but can\'t be read is refused as Node refuses it, and one that leads nowhere is none, on disk and in a Vfs', withTmp((t, tmp) => { + const conditions = new Set(['require']) + const outcome = (f) => { + try { + return f() + } catch (err) { + return err.code + } + } + // On disk, beside Node's own resolver: a link loop, a loop through directories, a link to nothing. + writeFileSync(join(tmp, 'package.json'), '{"name":"outer"}') + writeFileSync(join(tmp, 'main.js'), '') + for (const dir of ['loop', 'dirloop', 'dangling']) { + mkdirSync(join(tmp, dir)) + writeFileSync(join(tmp, dir, 'index.js'), '') + } + symlinkSync('package.json', join(tmp, 'loop/package.json')) + symlinkSync('b', join(tmp, 'dirloop/a')) + symlinkSync('a', join(tmp, 'dirloop/b')) + symlinkSync('a/x.json', join(tmp, 'dirloop/package.json')) + symlinkSync('gone.json', join(tmp, 'dangling/package.json')) + const ours = createNodeResolver(diskHost) + const node = createRequire(join(tmp, 'main.js')) + for (const dir of ['loop', 'dirloop', 'dangling']) { + t.assert.equal(outcome(() => ours.resolve(join(tmp, 'main.js'), `./${dir}`, conditions)), outcome(() => node.resolve(`./${dir}`)), dir) + } + t.assert.equal(outcome(() => ours.resolve(join(tmp, 'main.js'), './loop', conditions)), 'ERR_INVALID_PACKAGE_CONFIG') + // In a Vfs, as on disk. + const vfs = write(new Vfs(), { '/package.json': '{"name":"outer"}', '/main.js': '', '/loop/index.js': '', '/dangling/index.js': '' }) + vfs.symlink('package.json', '/loop/package.json') + vfs.symlink('gone.json', '/dangling/package.json') + const host = createVfsHost(vfs) + t.assert.throws(() => host.findPackageJSON('/loop/index.js'), { code: 'ERR_INVALID_PACKAGE_CONFIG' }) + t.assert.throws(() => host.resolve('/main.js', './loop', conditions), { code: 'ERR_INVALID_PACKAGE_CONFIG' }) + t.assert.equal(host.findPackageJSON('/dangling/index.js'), '/package.json') + t.assert.equal(host.resolve('/main.js', './dangling', conditions), '/dangling/index.js') +})) + test('the disk host resolves exactly like require.resolve, including through symlinks', withTmp((t, tmp) => { mkdirSync(join(tmp, 'real')) writeFileSync(join(tmp, 'real', 'z.js'), '') From d951c4b42f653fee53787e3889bd11bd956e18f1 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 09:01:37 +0000 Subject: [PATCH 3/3] fix(bundle): invalid-remapping errors name the entry, never its text MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An invalid remapping's error quoted it. The text is normally a remapping the project wrote, but a file named as a mapping by mistake can hold anything: `--mapping=token.txt` printed `token.txt:1: invalid remapping "ghp_…"` to stderr, and into CI logs with it. A token pasted into FOUNDRY_REMAPPINGS did the same. Errors now give the location and the form the entry should have: `remappings.txt:2: invalid remapping, expected [context:]prefix=target`. A foundry.toml entry is named by its position (`` `remappings` entry 2 ``, `` `remappings` entry 1 is not a string ``). This is the last error of stasis's own that quoted file content. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01C6oBS5QX4oqZcd2d3STiGA --- doc/file-formats.md | 3 ++- stasis/src/loaders/foundry.js | 15 ++++++++++----- tests/bundle-cmd.test.js | 14 +++++++++----- tests/solidity-loader.test.js | 12 ++++++------ 4 files changed, 27 insertions(+), 17 deletions(-) diff --git a/doc/file-formats.md b/doc/file-formats.md index 5de2b6c8..1baeba92 100644 --- a/doc/file-formats.md +++ b/doc/file-formats.md @@ -369,7 +369,8 @@ 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 diff --git a/stasis/src/loaders/foundry.js b/stasis/src/loaders/foundry.js index 38e7916d..0dcf5dfb 100644 --- a/stasis/src/loaders/foundry.js +++ b/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 }) } diff --git a/tests/bundle-cmd.test.js b/tests/bundle-cmd.test.js index 8ccbff7b..a467e74c 100644 --- a/tests/bundle-cmd.test.js +++ b/tests/bundle-cmd.test.js @@ -823,7 +823,7 @@ test('buildSolidityBundle fails on a foundry.toml that isn\'t TOML, naming the f )) })) -test('buildSolidityBundle fails on an invalid remapping, the project\'s or a dependency\'s, naming the file and line', withTmp(async (t, tmp) => { +test('buildSolidityBundle fails on an invalid remapping, the project\'s or a dependency\'s, naming the file and line but never quoting it', withTmp(async (t, tmp) => { writeProject(tmp, { 'foundry.toml': '[profile.default]\n', 'remappings.txt': 'dep/=lib/dep/src/\n# not a remapping\n', @@ -831,15 +831,19 @@ test('buildSolidityBundle fails on an invalid remapping, the project\'s or a dep 'lib/dep/src/D.sol': 'contract D {}\n', }) const fails = (opts, message) => captureStderr(() => t.assert.rejects(() => buildSolidityBundle({ cwd: tmp, entries: ['src'], env: {}, ...opts }), { message })) - await fails({}, 'remappings.txt:2: invalid remapping "# not a remapping"') + await fails({}, 'remappings.txt:2: invalid remapping, expected [context:]prefix=target') // As written for solc, and as a pinned mapping file, alike. - await fails({ mappingFile: 'remappings.txt' }, 'remappings.txt:2: invalid remapping "# not a remapping"') + await fails({ mappingFile: 'remappings.txt' }, 'remappings.txt:2: invalid remapping, expected [context:]prefix=target') + // A file named as a mapping by mistake: what it holds isn't echoed, a secret included. + writeFileSync(join(tmp, 'token.txt'), 'ghp_0123456789abcdefSECRET\n') + await fails({ mappingFile: 'token.txt' }, 'token.txt:1: invalid remapping, expected [context:]prefix=target') + await fails({ env: { FOUNDRY_REMAPPINGS: 'sk-live-SECRET' } }, 'FOUNDRY_REMAPPINGS:1: invalid remapping, expected [context:]prefix=target') writeFileSync(join(tmp, 'remappings.txt'), 'dep/=lib/dep/src/\n') // forge skips a dependency's config holding one; here it's an error, not a config left out. writeProject(tmp, { 'lib/dep/foundry.toml': '[profile.default]\nremappings = ["x"]\n' }) - await fails({}, 'lib/dep/foundry.toml: `remappings`: invalid remapping "x"') + await fails({}, 'lib/dep/foundry.toml: `remappings` entry 1: invalid remapping, expected [context:]prefix=target') writeProject(tmp, { 'lib/dep/foundry.toml': '[profile.default]\n', 'lib/dep/remappings.txt': 'y/=src/\n=z\n' }) - await fails({}, 'lib/dep/remappings.txt:2: invalid remapping "=z"') + await fails({}, 'lib/dep/remappings.txt:2: invalid remapping, expected [context:]prefix=target') writeFileSync(join(tmp, 'lib/dep/remappings.txt'), 'y/=src/\n') const bundle = await buildSolidityBundle({ cwd: tmp, entries: ['src'], env: {} }) t.assert.deepEqual([...bundle.sources.keys()].toSorted(), ['lib/dep/src/D.sol', 'src/A.sol']) diff --git a/tests/solidity-loader.test.js b/tests/solidity-loader.test.js index 15b12607..4ff3483b 100644 --- a/tests/solidity-loader.test.js +++ b/tests/solidity-loader.test.js @@ -82,10 +82,10 @@ test('parseRemappings handles one-per-line entries and refuses an invalid line, { context: null, prefix: '@a/', target: 'lib/a/' }, { context: null, prefix: '@b/', target: 'lib/b/' }, ]) - t.assert.throws(() => parseRemappings('@a/=lib/a/\ngarbage line\n'), { message: 'remappings:2: invalid remapping "garbage line"' }) + t.assert.throws(() => parseRemappings('@a/=lib/a/\ngarbage line\n'), { message: 'remappings:2: invalid remapping, expected [context:]prefix=target' }) // Lines are trimmed as Rust trims them: a byte-order mark isn't whitespace, and stays. t.assert.deepEqual(parseRemappings('\uFEFFx/=a/\n'), [{ context: null, prefix: '\uFEFFx/', target: 'a/' }]) - t.assert.throws(() => parseRemappings('\n=empty-prefix\n'), { message: 'remappings:2: invalid remapping "=empty-prefix"' }) + t.assert.throws(() => parseRemappings('\n=empty-prefix\n'), { message: 'remappings:2: invalid remapping, expected [context:]prefix=target' }) }) test('parseRemappings reads a `context:` before the prefix', (t) => { @@ -839,11 +839,11 @@ test('an invalid remapping in a foundry.toml or remappings variable is an error, 'num/foundry.toml': '[profile.default]\nremappings = [1]\n', 'ok/foundry.toml': '[profile.default]\n', }, (t, dir) => { - t.assert.throws(() => foundryProject(dir, { env: {} }), { message: 'foundry.toml: `remappings`: invalid remapping "nope"' }) + t.assert.throws(() => foundryProject(dir, { env: {} }), { message: 'foundry.toml: `remappings` entry 2: invalid remapping, expected [context:]prefix=target' }) t.assert.throws(() => foundryProject(join(dir, 'list'), { env: {} }), { message: 'foundry.toml: `remappings` is not an array of strings' }) - t.assert.throws(() => foundryProject(join(dir, 'num'), { env: {} }), { message: 'foundry.toml: `remappings`: invalid remapping 1' }) - t.assert.throws(() => foundryProject(join(dir, 'ok'), { env: { FOUNDRY_REMAPPINGS: 'x/=y/\nbad' } }), { message: 'FOUNDRY_REMAPPINGS:2: invalid remapping "bad"' }) - t.assert.throws(() => foundryTomlRemappings('[profile.default]\nremappings = ["=x/"]\n'), { message: '`remappings`: invalid remapping "=x/"' }) + t.assert.throws(() => foundryProject(join(dir, 'num'), { env: {} }), { message: 'foundry.toml: `remappings` entry 1 is not a string' }) + t.assert.throws(() => foundryProject(join(dir, 'ok'), { env: { FOUNDRY_REMAPPINGS: 'x/=y/\nbad' } }), { message: 'FOUNDRY_REMAPPINGS:2: invalid remapping, expected [context:]prefix=target' }) + t.assert.throws(() => foundryTomlRemappings('[profile.default]\nremappings = ["=x/"]\n'), { message: '`remappings` entry 1: invalid remapping, expected [context:]prefix=target' }) })) test('a legacy [default] table\'s `extends` is ignored, as forge ignores it', withProject({