Skip to content

Commit c80a562

Browse files
committed
solidity loader: resolve @-scoped imports via Node-style walk-up
The hardcoded `node_modules/<spec>` fallback couldn't see monorepos, didn't verify the package existed, and accepted `..` segments verbatim (letting a malicious solidity source read sibling files). Replace it with `resolveNodeModulesSpec(baseDir, specifier)`: - splits `@scope/pkg/sub/path` correctly - walks up from baseDir looking for `<dir>/node_modules/<pkg>/package.json` - rejects `..` in the subpath - rejects resolutions outside baseDir (the Bundle layer can't store them) `resolveSolImport` is pure again. The Node fallback is applied at both call sites (`collectSolidityFilesFromDisk` already has baseDir; `buildSolidityTree` now accepts it as an option, threaded through from `buildSolidityBundle`).
1 parent 11e5102 commit c80a562

8 files changed

Lines changed: 121 additions & 3 deletions

File tree

src/cmd/bundle.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ export async function buildSolidityBundle({ cwd = process.cwd(), entries, mappin
9292
const normalized = normalizeEntries(entries, cwd)
9393

9494
const sources = await collectSolidityFilesFromDisk(baseDir, normalized, remappings)
95-
const { resolutions, missing } = buildSolidityTree(sources, { remappings })
95+
const { resolutions, missing } = buildSolidityTree(sources, { remappings, baseDir })
9696

9797
// Bundles must be self-contained. Refuse to write one when an entry
9898
// can't be loaded from disk, or when any in-bundle file has an import

src/loaders/solidity.js

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
// file (foundry.toml or remappings.txt) is read to extract `remappings`,
66
// but is itself not included in `sources`.
77

8+
import { existsSync } from 'node:fs'
89
import { readFile } from 'node:fs/promises'
910
import { dirname, isAbsolute, join, relative, resolve } from 'node:path'
1011

@@ -73,6 +74,41 @@ export function resolveSolImport(specifier, fromFile, remappings) {
7374
return null
7475
}
7576

77+
// Node-style resolution for a bare `@scope/pkg/sub/path.sol` specifier:
78+
// walk up from `baseDir` looking for `<dir>/node_modules/<pkg>/package.json`
79+
// (matching Node's CJS resolution algorithm). Returns the file path
80+
// relative to `baseDir` (POSIX-style), or null when no such package is
81+
// found, when the subpath contains `..`, or when the package was found
82+
// outside `baseDir` (a parent monorepo's node_modules) and bundling it
83+
// would require an out-of-tree path the Bundle layer would reject.
84+
export function resolveNodeModulesSpec(baseDir, specifier) {
85+
if (!specifier.startsWith('@')) return null
86+
const parts = specifier.split('/')
87+
if (parts.length < 3) return null // `@scope/pkg` alone has no file to load
88+
const pkgName = parts.slice(0, 2).join('/')
89+
const subParts = parts.slice(2)
90+
// Path-traversal guard: a `..` in the subpath could escape the package
91+
// directory (or even baseDir) once we hand the joined path to readFile.
92+
if (subParts.includes('..')) return null
93+
const subPath = subParts.join('/')
94+
let dir = resolve(baseDir)
95+
while (true) {
96+
const pkgDir = join(dir, 'node_modules', pkgName)
97+
if (existsSync(join(pkgDir, 'package.json'))) {
98+
const abs = join(pkgDir, subPath)
99+
const rel = relative(baseDir, abs).split(/[\\/]/u).join('/')
100+
// The Bundle workspace bucket rejects file paths that start with
101+
// `..`, so a package resolved at a parent's node_modules can't
102+
// round-trip through the bundle even though Node could load it.
103+
if (rel.startsWith('..') || isAbsolute(rel)) return null
104+
return rel
105+
}
106+
const parent = dirname(dir)
107+
if (parent === dir) return null
108+
dir = parent
109+
}
110+
}
111+
76112
// Build the { sources, resolutions, missing } triple from a Map of
77113
// already-loaded Solidity sources (any key format — relative paths for
78114
// disk-loaded projects, `${address}/${cname}` for etherscan, etc.) plus
@@ -82,13 +118,17 @@ export function resolveSolImport(specifier, fromFile, remappings) {
82118
// `@openzeppelin/contracts/.../IERC20.sol` which is itself a stored
83119
// cname). `missing` lists every (spec, from) pair that could not be
84120
// resolved or that resolved to a file that wasn't loaded into `sources`.
85-
export function buildSolidityTree(sources, { remappings = [] } = {}) {
121+
export function buildSolidityTree(sources, { remappings = [], baseDir } = {}) {
86122
const resolutions = new Map()
87123
const missing = []
88124
for (const [path, content] of sources) {
89125
const specMap = new Map()
90126
for (const spec of extractSolImports(content)) {
91127
let resolved = resolveSolImport(spec, path, remappings)
128+
// Node-style node_modules fallback for `@scope/pkg/...` specifiers
129+
// when remappings/relative resolution didn't hit. Requires baseDir
130+
// since we have to walk disk to find the package.
131+
if (!resolved && baseDir) resolved = resolveNodeModulesSpec(baseDir, spec)
92132
if (resolved && !sources.has(resolved)) resolved = null
93133
// Verbatim fallback: many Solidity ecosystems (etherscan bundles,
94134
// hardhat flat layouts) ship `@openzeppelin/...` imports as literal
@@ -152,6 +192,7 @@ export async function collectSolidityFilesFromDisk(baseDir, entries, remappings)
152192
sources.set(relPath, content)
153193
for (const spec of extractSolImports(content)) {
154194
let resolved = resolveSolImport(spec, relPath, remappings)
195+
if (!resolved) resolved = resolveNodeModulesSpec(baseDir, spec)
155196
if (!resolved && knownEntries.has(spec)) resolved = spec
156197
if (resolved) {
157198
if (!sources.has(resolved)) next.push(resolved)

tests/bundle-cmd.test.js

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,23 @@ test('buildSolidityBundle places node_modules files in per-package modules bucke
130130
)
131131
})
132132

133+
test('buildSolidityBundle resolves @-scoped imports via node_modules with no mapping file', async (t) => {
134+
// No --mapping passed: @oz/contracts/utils/Math.sol must fall back to
135+
// node_modules/@oz/contracts/utils/Math.sol on disk.
136+
const cwd = join(fixtures, 'nm-fallback')
137+
const bundle = await buildSolidityBundle({ cwd, entries: ['src/A.sol'] })
138+
t.assert.deepEqual(
139+
[...bundle.modules.keys()].toSorted(),
140+
['.', 'node_modules/@oz/contracts'],
141+
)
142+
t.assert.equal(bundle.modules.get('node_modules/@oz/contracts').name, '@oz/contracts')
143+
t.assert.equal(bundle.modules.get('node_modules/@oz/contracts').version, '5.0.0')
144+
t.assert.equal(
145+
bundle.imports.get('solidity').get('src/A.sol').get('@oz/contracts/utils/Math.sol'),
146+
'node_modules/@oz/contracts/utils/Math.sol',
147+
)
148+
})
149+
133150
test('buildSolidityBundle throws when a node_modules file has no resolvable package.json', async (t) => {
134151
await t.assert.rejects(
135152
() => buildSolidityBundle({

tests/fixtures/solidity-bundle/nm-fallback/node_modules/@oz/contracts/package.json

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

tests/fixtures/solidity-bundle/nm-fallback/node_modules/@oz/contracts/utils/Math.sol

Lines changed: 8 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"name": "my-app",
3+
"version": "0.1.0"
4+
}
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
// SPDX-License-Identifier: MIT
2+
pragma solidity ^0.8.0;
3+
4+
import "@oz/contracts/utils/Math.sol";
5+
6+
contract A {
7+
function foo() public pure returns (uint256) {
8+
return Math.max(1, 2);
9+
}
10+
}

tests/solidity-loader.test.js

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
parseRemappings,
1212
parseRemappingsFromToml,
1313
readRemappingsFile,
14+
resolveNodeModulesSpec,
1415
resolveSolImport,
1516
} from '../src/loaders/solidity.js'
1617

@@ -86,8 +87,41 @@ test('resolveSolImport picks the longest remapping prefix', (t) => {
8687
t.assert.equal(resolveSolImport('@oz/other.sol', 'src/A.sol', remappings), 'lib/oz/other.sol')
8788
})
8889

89-
test('resolveSolImport returns null for non-relative, non-remapped imports', (t) => {
90+
test('resolveSolImport returns null for non-relative, non-remapped imports (no disk lookup)', (t) => {
91+
// The pure resolver doesn't know about disk; the caller layers a
92+
// resolveNodeModulesSpec fallback on top when baseDir is available.
9093
t.assert.equal(resolveSolImport('@unknown/Foo.sol', 'src/A.sol', []), null)
94+
t.assert.equal(resolveSolImport('foo/X.sol', 'src/A.sol', []), null)
95+
})
96+
97+
test('resolveNodeModulesSpec finds a scoped package at baseDir/node_modules and returns the subpath', (t) => {
98+
const baseDir = join(fixtures, 'nm-fallback')
99+
t.assert.equal(
100+
resolveNodeModulesSpec(baseDir, '@oz/contracts/utils/Math.sol'),
101+
'node_modules/@oz/contracts/utils/Math.sol',
102+
)
103+
})
104+
105+
test('resolveNodeModulesSpec returns null for unscoped specifiers (only `@scope/pkg/...` is supported)', (t) => {
106+
t.assert.equal(resolveNodeModulesSpec(join(fixtures, 'nm-fallback'), 'foo/X.sol'), null)
107+
t.assert.equal(resolveNodeModulesSpec(join(fixtures, 'nm-fallback'), 'X.sol'), null)
108+
})
109+
110+
test('resolveNodeModulesSpec returns null for `@scope/pkg` with no file subpath', (t) => {
111+
t.assert.equal(resolveNodeModulesSpec(join(fixtures, 'nm-fallback'), '@oz/contracts'), null)
112+
})
113+
114+
test('resolveNodeModulesSpec rejects `..` in the subpath (path-traversal guard)', (t) => {
115+
const baseDir = join(fixtures, 'nm-fallback')
116+
t.assert.equal(resolveNodeModulesSpec(baseDir, '@oz/contracts/../../etc/passwd'), null)
117+
t.assert.equal(resolveNodeModulesSpec(baseDir, '@oz/contracts/utils/../Math.sol'), null)
118+
})
119+
120+
test('resolveNodeModulesSpec returns null when the package is not installed under baseDir', (t) => {
121+
t.assert.equal(
122+
resolveNodeModulesSpec(join(fixtures, 'nm-fallback'), '@absent/nope/X.sol'),
123+
null,
124+
)
91125
})
92126

93127
test('resolveSolImport returns null when relative traversal escapes the root', (t) => {

0 commit comments

Comments
 (0)