The first real module. It registers nothing, deliberately: what slice 0 proves
is the delivery path itself, end to end, before a single UO file moves into it.
Server half: module.json, an entry point that takes (ctx, api) and registers
nothing, a test suite built on a fake ctx, and scripts/checkImports.js -- the
MODULE_API.md §5.1 boundary check. Client half: the Vite library build, four
shims re-exporting react / react-dom/client / react-router-dom / jsx-runtime
from window.__rg, an entry that verifies each is identity-equal to core's copy,
and scripts/checkExternals.js. 29 server tests, 9 client tests, both new.
Verified against a real core: the module loads, mounts its zero routes, runs to
`started`, and is published by /api/v1/public/modules. Its chunk serves from
the entry's directory with `Cache-Control: no-cache` while the module's server
source, module.json and package.json all 404. In Chrome, under the enforced
`script-src 'self'`, the chunk evaluates and reports all four shared
dependencies OK, with zero CSP reports and no console errors.
Three findings, each of which had produced a green build that was wrong.
MODULE_API.md §3.6 shows `external` alongside the aliases and they do not
compose. Rollup asks `external` BEFORE Vite's alias resolver runs, so a
specifier in both is marked external and never aliased -- the chunk then ships
bare `import "react"`, which no browser can resolve without an import map, and
CSP forbids one. Built cleanly and emitted exactly that; checkExternals caught
it. So: alias only, `external` empty, and vite.config.js grows a resolution-time
guard that fails the build if a shared dependency resolves into node_modules.
That guard was wrong twice before it worked. Written against Rollup's `load`
hook it never ran -- `load` is first-wins and an earlier plugin had already
claimed the module -- so a deliberately-broken alias produced a 24 kB chunk with
react-router welded in, and a green build. And its forbidden-package list was
derived from the alias list "so the two cannot disagree", which meant deleting
an alias also deleted the guard against what that alias prevented. It states the
contract now, and a test asserts the aliases stay inside it.
checkImports failed on its own documentation the first time it ran: the comment
naming require("../../etc/passwd") as an example of what to catch, and index.js
explaining why the module must never require("express"). A boundary check that
cannot survive being described is one people stop writing comments around. It
strips comments and template literals with a character walk rather than a
regexp, because a URL in a string contains a comment opener and a comment
contains quotes -- and it has its own test suite, since a check never shown to
fail is a check nobody knows the state of.
Co-Authored-By: Claude <noreply@anthropic.com>
177 lines
7.8 KiB
JavaScript
177 lines
7.8 KiB
JavaScript
#!/usr/bin/env node
|
|
// ── §5.1 — zero internal imports ───────────────────────────────────────────
|
|
//
|
|
// The acceptance test for the whole module contract. A module that reaches into
|
|
// core's tree still works — right up until core moves a file — and the boundary
|
|
// this workstream exists to build is worth exactly as much as this check is.
|
|
//
|
|
// MODULE_API.md §5.1 sketches it as a grep for `../../`. That is the shape of
|
|
// the violation but not the rule, and the difference matters in both directions:
|
|
// a grep says nothing about `require('../../../../etc/passwd')` from a deeply
|
|
// nested file (which it catches by accident) and false-alarms on a legitimate
|
|
// `require('../module.json')` from `server/` (which it catches wrongly). So this
|
|
// RESOLVES each specifier against the file that wrote it and asks whether the
|
|
// result is still inside the module root — the actual rule, stated once.
|
|
//
|
|
// Bare specifiers are checked too, and against a stricter list than "is it
|
|
// installed": core hands the module express, express-validator, the database and
|
|
// the logger on `ctx` precisely so the module never resolves them, and Node's
|
|
// resolver cannot reach core's `node_modules` from here anyway. A bare
|
|
// `require` that is not a Node builtin is therefore a module that will fail to
|
|
// load on a real install, with a message about a missing package rather than
|
|
// about the rule it broke.
|
|
//
|
|
// **That second check applies to SHIPPED code only.** `test/` and `scripts/`
|
|
// never run inside core's process — the fakes in `test/_fakes.js` build a real
|
|
// `express` router precisely so the module's routers are exercised for real —
|
|
// so they may use devDependencies. The containment check applies everywhere,
|
|
// because a test that reaches into core's tree is a test that passes on this
|
|
// machine and nowhere else.
|
|
//
|
|
// Run over the SERVER half. The client half's equivalent is its Vite build:
|
|
// the four shared dependencies are `external`, and anything else that stays a
|
|
// bare import in the emitted chunk is unresolvable in the browser.
|
|
|
|
const fs = require('fs')
|
|
const path = require('path')
|
|
const { builtinModules } = require('module')
|
|
|
|
const MODULE_ROOT = path.resolve(__dirname, '..', '..')
|
|
const SERVER_ROOT = path.join(MODULE_ROOT, 'server')
|
|
|
|
const BUILTINS = new Set([...builtinModules, ...builtinModules.map((m) => `node:${m}`)])
|
|
|
|
// Dependencies this half is allowed to resolve for itself. Empty, and that is
|
|
// the design: everything the server half needs comes from `ctx` (§2.3). A new
|
|
// entry here is a real decision — it becomes a package an operator's install
|
|
// has to carry — so it should be argued for in a PR, not added in passing.
|
|
const ALLOWED_PACKAGES = new Set([])
|
|
|
|
const SKIP_DIRS = new Set(['node_modules', 'coverage', '.git'])
|
|
|
|
// Directories whose contents never run inside core's process, and may therefore
|
|
// resolve this package's devDependencies.
|
|
const NOT_SHIPPED = [path.join(SERVER_ROOT, 'test'), path.join(SERVER_ROOT, 'scripts')]
|
|
const isShipped = (file) => !NOT_SHIPPED.some((d) => file.startsWith(d + path.sep))
|
|
|
|
const devDependencies = new Set(
|
|
Object.keys(JSON.parse(fs.readFileSync(path.join(SERVER_ROOT, 'package.json'), 'utf8')).devDependencies || {}),
|
|
)
|
|
|
|
// `require('x')`, `from 'x'`, `import('x')`. Deliberately textual: parsing would
|
|
// need a dependency, and a specifier this pattern misses is a specifier written
|
|
// to be missed, which review catches and a stricter regexp would not.
|
|
const SPECIFIER = /(?:require\(|from\s+|import\()\s*['"]([^'"]+)['"]/g
|
|
|
|
/**
|
|
* Blank out comments and template literals before scanning.
|
|
*
|
|
* Not a nicety — without it this file fails on ITSELF, because the comments
|
|
* above name `require('../../../../etc/passwd')` as an example of what to
|
|
* catch, and index.js explains in prose why it must never `require('express')`.
|
|
* A boundary check that cannot survive being described is a check people stop
|
|
* writing comments around.
|
|
*
|
|
* A character walk rather than a regexp, because the two get in each other's
|
|
* way: `'https://x'` contains a line-comment opener inside a string, and
|
|
* `// don't` contains a quote inside a comment. Tracking the state is shorter
|
|
* than the regexp that would almost handle it. Content is replaced with spaces
|
|
* rather than removed so nothing else has to care.
|
|
*/
|
|
function stripCommentsAndTemplates(src) {
|
|
let out = ''
|
|
let i = 0
|
|
const keep = (n) => { out += src.slice(i, i + n); i += n }
|
|
const blank = (end) => { out += src.slice(i, end).replace(/[^\n]/g, ' '); i = end }
|
|
while (i < src.length) {
|
|
const two = src.slice(i, i + 2)
|
|
if (two === '//') {
|
|
const nl = src.indexOf('\n', i)
|
|
blank(nl === -1 ? src.length : nl)
|
|
} else if (two === '/*') {
|
|
const end = src.indexOf('*/', i + 2)
|
|
blank(end === -1 ? src.length : end + 2)
|
|
} else if (src[i] === '"' || src[i] === "'") {
|
|
// Strings are KEPT — they are where the specifiers live.
|
|
const quote = src[i]
|
|
keep(1)
|
|
while (i < src.length && src[i] !== quote) keep(src[i] === '\\' ? 2 : 1)
|
|
keep(1)
|
|
} else if (src[i] === '`') {
|
|
// Template literals are blanked: nothing may `require` a template, and a
|
|
// template holding SQL or HTML is a rich source of false positives.
|
|
i += 1
|
|
out += ' '
|
|
while (i < src.length && src[i] !== '`') {
|
|
if (src[i] === '\\') { out += ' '; i += 2 } else { out += src[i] === '\n' ? '\n' : ' '; i += 1 }
|
|
}
|
|
i += 1
|
|
out += ' '
|
|
} else {
|
|
keep(1)
|
|
}
|
|
}
|
|
return out
|
|
}
|
|
|
|
function* walk(dir) {
|
|
for (const entry of fs.readdirSync(dir, { withFileTypes: true })) {
|
|
if (entry.isDirectory()) {
|
|
if (!SKIP_DIRS.has(entry.name)) yield* walk(path.join(dir, entry.name))
|
|
} else if (/\.(js|mjs|cjs)$/.test(entry.name)) {
|
|
yield path.join(dir, entry.name)
|
|
}
|
|
}
|
|
}
|
|
|
|
/**
|
|
* Every boundary violation under `root`, resolved against `moduleRoot`.
|
|
*
|
|
* Exported so `test/checkImports.test.js` can point it at fixtures. A check that
|
|
* has never been shown to fail is a check nobody knows the state of — and this
|
|
* one guards the acceptance criterion for the whole contract.
|
|
*/
|
|
function scan(root, moduleRoot = MODULE_ROOT, { shipped = isShipped, dev = devDependencies } = {}) {
|
|
const violations = []
|
|
for (const file of walk(root)) {
|
|
const source = stripCommentsAndTemplates(fs.readFileSync(file, 'utf8'))
|
|
for (const [, specifier] of source.matchAll(SPECIFIER)) {
|
|
if (specifier.startsWith('.')) {
|
|
const resolved = path.resolve(path.dirname(file), specifier)
|
|
if (resolved !== moduleRoot && !resolved.startsWith(moduleRoot + path.sep)) {
|
|
violations.push({ file, specifier, why: 'escapes the module root' })
|
|
}
|
|
} else if (path.isAbsolute(specifier)) {
|
|
violations.push({ file, specifier, why: 'absolute path' })
|
|
} else {
|
|
const pkg = specifier.startsWith('@')
|
|
? specifier.split('/').slice(0, 2).join('/')
|
|
: specifier.split('/')[0]
|
|
const allowed = ALLOWED_PACKAGES.has(pkg) || (!shipped(file) && dev.has(pkg))
|
|
if (!BUILTINS.has(specifier) && !BUILTINS.has(pkg) && !allowed) {
|
|
violations.push({ file, specifier, why: 'undeclared bare specifier — should this come from ctx?' })
|
|
}
|
|
}
|
|
}
|
|
}
|
|
return violations
|
|
}
|
|
|
|
module.exports = { scan, stripCommentsAndTemplates, SERVER_ROOT, MODULE_ROOT }
|
|
|
|
// Required by a test, or run as the check? Only the second one exits.
|
|
if (require.main !== module) return
|
|
|
|
const violations = scan(SERVER_ROOT)
|
|
|
|
if (violations.length) {
|
|
console.error(`\n${violations.length} import(s) break the module boundary (MODULE_API.md §5.1):\n`)
|
|
for (const v of violations) {
|
|
console.error(` ${path.relative(MODULE_ROOT, v.file)}\n "${v.specifier}" — ${v.why}`)
|
|
}
|
|
console.error('')
|
|
process.exit(1)
|
|
}
|
|
|
|
console.log(`OK — no import escapes the module root (${SERVER_ROOT}).`)
|