Refuse a node_modules at the plugins/ root, where it outranks the host's
CI / full-gate (push) Successful in 2m43s

This commit is contained in:
lilleman
2026-08-18 08:14:21 +02:00
parent 3f9787df48
commit 7d1f7750d3
5 changed files with 18 additions and 17 deletions
+1 -2
View File
@@ -6,8 +6,7 @@ npm-debug.log
*.log *.log
.DS_Store .DS_Store
# A plugin author's pin setting, per README → Plugin dependencies. Nothing reads it at runtime, and # A plugin's .npmrc is where a private-registry token would sit — never in a shipped image.
# an .npmrc is where a private-registry token would sit — never bake one into a shipped image.
plugins/**/.npmrc plugins/**/.npmrc
e2e-tests/artifacts e2e-tests/artifacts
+2 -2
View File
@@ -709,8 +709,8 @@ included, since the plugin folder *is* the repo. A baked image needs no extra st
- **Never ship a copy of `@plainpages/plugin-api`.** The host publishes it into `/node_modules`, - **Never ship a copy of `@plainpages/plugin-api`.** The host publishes it into `/node_modules`,
above every plugin, and a plugin resolves it from there — nothing to declare, just import it. A above every plugin, and a plugin resolves it from there — nothing to declare, just import it. A
copy inside your own `node_modules` would shadow it with a *second* instance of the host's copy inside your plugin's own `node_modules` would shadow it with a *second* instance of the host's
contract, turning a sign-in redirect into a 500, so discovery refuses one at boot. contract, turning a sign-in redirect into a 500, so discovery refuses one there at boot.
- **The host never upgrades or dedupes your dependencies.** Two plugins depending on the same package - **The host never upgrades or dedupes your dependencies.** Two plugins depending on the same package
each get their own copy at their own version, so neither can break the other by upgrading — and each get their own copy at their own version, so neither can break the other by upgrading — and
keeping yours current, and audited, is yours to own. Renovate here watches the host's manifests keeping yours current, and audited, is yours to own. Renovate here watches the host's manifests
+2 -2
View File
@@ -31,8 +31,8 @@ export interface RequestContext {
// on off-site URLs. The host already does this for the chrome and its own redirects; a plugin // on off-site URLs. The host already does this for the chrome and its own redirects; a plugin
// wraps the hrefs it builds itself. // wraps the hrefs it builds itself.
localeHref(href: string): string; localeHref(href: string): string;
// Every installed locale, sorted. With `localeLabel` (from the barrel) it is what a plugin needs // Every installed locale, sorted. With `localeLabel` (from @plainpages/plugin-api) it is what a
// to build its own language picker; the host's own picker is already in the shell. // plugin needs to build its own language picker; the host's own picker is already in the shell.
locales: string[]; locales: string[];
// Request-scoped logger: structured, in the request's trace. `log.info/warn/error(...)` to // Request-scoped logger: structured, in the request's trace. `log.info/warn/error(...)` to
// log; `log.fetch(url)` for an upstream call (a client span continuing the trace). Correlates by // log; `log.fetch(url)` for an upstream call (a client span continuing the trace). Correlates by
+4 -5
View File
@@ -61,9 +61,9 @@ const badCases: Array<{ name: string; files: Record<string, string>; match: RegE
{ name: "a plugin package.json that forgets type: module", files: { "cjs/package.json": `{ "name": "cjs" }`, "cjs/plugin.ts": full("cjs") }, match: /cjs.*"type": "module"/s }, { name: "a plugin package.json that forgets type: module", files: { "cjs/package.json": `{ "name": "cjs" }`, "cjs/plugin.ts": full("cjs") }, match: /cjs.*"type": "module"/s },
{ name: "a plugin package.json that is not valid JSON", files: { "bent/package.json": `{`, "bent/plugin.ts": full("bent") }, match: /bent.*package\.json.*JSON/s }, { name: "a plugin package.json that is not valid JSON", files: { "bent/package.json": `{`, "bent/plugin.ts": full("bent") }, match: /bent.*package\.json.*JSON/s },
{ name: "a plugin package.json holding null", files: { "nul/package.json": `null`, "nul/plugin.ts": full("nul") }, match: /nul.*"type": "module"/s }, { name: "a plugin package.json holding null", files: { "nul/package.json": `null`, "nul/plugin.ts": full("nul") }, match: /nul.*"type": "module"/s },
// Written by the documented install command with one path segment dropped — and it would silently // `npm install --prefix plugins` — the documented command with one path segment dropped.
// make every plugin below it part of its scope.
{ name: "a package.json in the scan root itself", files: { "package.json": `{ "name": "oops" }`, "ok/plugin.ts": full("ok") }, match: /plugins\/package\.json must not exist/ }, { name: "a package.json in the scan root itself", files: { "package.json": `{ "name": "oops" }`, "ok/plugin.ts": full("ok") }, match: /plugins\/package\.json must not exist/ },
{ name: "a node_modules in the scan root itself", files: { "node_modules/@plainpages/plugin-api/index.js": `export class GuardError extends Error {}`, "ok/plugin.ts": full("ok") }, match: /plugins\/node_modules must not exist/ },
{ name: "two plugins claim the public home", files: { "a/plugin.ts": `export default { apiVersion: "1.0.0", home: () => ({ html: "a" }) };`, "b/plugin.ts": `export default { apiVersion: "1.0.0", home: () => ({ html: "b" }) };` }, match: /home/ }, { name: "two plugins claim the public home", files: { "a/plugin.ts": `export default { apiVersion: "1.0.0", home: () => ({ html: "a" }) };`, "b/plugin.ts": `export default { apiVersion: "1.0.0", home: () => ({ html: "b" }) };` }, match: /home/ },
{ name: "two plugins claim the gated dashboard", files: { "a/plugin.ts": `export default { apiVersion: "1.0.0", dashboard: () => ({ html: "a" }) };`, "b/plugin.ts": `export default { apiVersion: "1.0.0", dashboard: () => ({ html: "b" }) };` }, match: /dashboard/ }, { name: "two plugins claim the gated dashboard", files: { "a/plugin.ts": `export default { apiVersion: "1.0.0", dashboard: () => ({ html: "a" }) };`, "b/plugin.ts": `export default { apiVersion: "1.0.0", dashboard: () => ({ html: "b" }) };` }, match: /dashboard/ },
]; ];
@@ -118,16 +118,15 @@ test("a plugin may carry its own package.json, node_modules and dependencies", a
"shop/node_modules/price-tag/index.js": `export default (n) => \`\${n} kr\`;`, "shop/node_modules/price-tag/index.js": `export default (n) => \`\${n} kr\`;`,
"shop/plugin.ts": `import { definePlugin } from "@plainpages/plugin-api";\nimport price from "price-tag";\n` + "shop/plugin.ts": `import { definePlugin } from "@plainpages/plugin-api";\nimport price from "price-tag";\n` +
`export default definePlugin({ apiVersion: "1.0.0", routes: [{ method: "GET", path: "/", handler: () => ({ html: price(20) }) }] });`, `export default definePlugin({ apiVersion: "1.0.0", routes: [{ method: "GET", path: "/", handler: () => ({ html: price(20) }) }] });`,
"node_modules/hoisted/index.js": `export default 1;`,
}); });
const plugins = await discoverPlugins({ dir }); const plugins = await discoverPlugins({ dir });
assert.deepEqual(plugins.map((p) => p.id), ["shop"]); // node_modules is not a plugin folder assert.deepEqual(plugins.map((p) => p.id), ["shop"]);
assert.deepEqual(await plugins[0]?.routes?.[0]?.handler(null as never), { html: "20 kr" }); assert.deepEqual(await plugins[0]?.routes?.[0]?.handler(null as never), { html: "20 kr" });
}); });
test("a plugin folder may be a symlink — a plugin kept in its own repo", async (t) => { test("a plugin folder may be a symlink", async (t) => {
const ownRepo = scaffold(t, { "my-plugin/plugin.ts": full("my-plugin") }); const ownRepo = scaffold(t, { "my-plugin/plugin.ts": full("my-plugin") });
const dir = scaffold(t, {}); const dir = scaffold(t, {});
symlinkSync(join(ownRepo, "my-plugin"), join(dir, "linked")); symlinkSync(join(ownRepo, "my-plugin"), join(dir, "linked"));
+9 -6
View File
@@ -27,8 +27,12 @@ export async function discoverPlugins(options: DiscoverOptions = {}): Promise<Pl
const errors: string[] = []; const errors: string[] = [];
const plugins: Plugin[] = []; const plugins: Plugin[] = [];
if (existsSync(join(dir, "package.json"))) { // `npm install --prefix plugins` instead of `--prefix plugins/<id>`: the package.json becomes the
errors.push(`plugins/package.json must not exist — it becomes the package scope for every plugin below it; install into plugins/<id>, not plugins/`); // scope for every plugin below it, and the node_modules outranks the host's own — barrel included.
for (const stray of ["node_modules", "package.json"]) {
if (existsSync(join(dir, stray))) {
errors.push(`plugins/${stray} must not exist — it sits above every plugin and shadows the host's own; delete plugins/{node_modules,package.json,package-lock.json} and install into plugins/<id>`);
}
} }
for (const id of pluginFolders(dir)) { for (const id of pluginFolders(dir)) {
@@ -83,9 +87,8 @@ export async function discoverPlugins(options: DiscoverOptions = {}): Promise<Pl
return plugins; return plugins;
} }
// Subfolders of plugins/, sorted for deterministic load order + stable conflict messages. Hidden // Sorted for deterministic load order + stable conflict messages. A symlink counts as a folder, and
// entries, plain files and node_modules are skipped; a symlink counts, so a plugin kept in its own // one whose target the container cannot see trips "no plugin.ts found" rather than vanishing.
// repo joins the tree with `ln -s`, and a dangling one trips "no plugin.ts found" rather than vanishing.
function pluginFolders(dir: string): string[] { function pluginFolders(dir: string): string[] {
return readdirSync(dir, { withFileTypes: true }) return readdirSync(dir, { withFileTypes: true })
.filter((e) => (e.isDirectory() || e.isSymbolicLink()) && !e.name.startsWith(".") && e.name !== "node_modules") .filter((e) => (e.isDirectory() || e.isSymbolicLink()) && !e.name.startsWith(".") && e.name !== "node_modules")
@@ -94,7 +97,7 @@ function pluginFolders(dir: string): string[] {
} }
// A barrel copy resolves before the host's, so its GuardError matches no `instanceof` here and a // A barrel copy resolves before the host's, so its GuardError matches no `instanceof` here and a
// sign-in redirect becomes a 500. A typeless folder re-parses every file it loads, and warns on each. // sign-in redirect becomes a 500.
function packagingError(folder: string): string | null { function packagingError(folder: string): string | null {
if (existsSync(join(folder, "node_modules", "@plainpages", "plugin-api"))) { if (existsSync(join(folder, "node_modules", "@plainpages", "plugin-api"))) {
return "ships its own copy of @plainpages/plugin-api — remove it; the host provides the one instance"; return "ships its own copy of @plainpages/plugin-api — remove it; the host provides the one instance";