From cb59eee76d35ac9300e4d32425647ab18bcfe0e7 Mon Sep 17 00:00:00 2001 From: lilleman Date: Tue, 18 Aug 2026 08:14:21 +0200 Subject: [PATCH] Refuse a node_modules at the plugins/ root, where it outranks the host's --- .dockerignore | 3 +-- README.md | 4 ++-- src/http/context.ts | 4 ++-- src/plugin-host/discovery.test.ts | 9 ++++----- src/plugin-host/discovery.ts | 15 +++++++++------ 5 files changed, 18 insertions(+), 17 deletions(-) diff --git a/.dockerignore b/.dockerignore index 2872634..4388e0c 100644 --- a/.dockerignore +++ b/.dockerignore @@ -6,8 +6,7 @@ npm-debug.log *.log .DS_Store -# A plugin author's pin setting, per README → Plugin dependencies. Nothing reads it at runtime, and -# an .npmrc is where a private-registry token would sit — never bake one into a shipped image. +# A plugin's .npmrc is where a private-registry token would sit — never in a shipped image. plugins/**/.npmrc e2e-tests/artifacts diff --git a/README.md b/README.md index 83efb62..1f20f27 100644 --- a/README.md +++ b/README.md @@ -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`, 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 - contract, turning a sign-in redirect into a 500, so discovery refuses one at boot. + 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 there at boot. - **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 keeping yours current, and audited, is yours to own. Renovate here watches the host's manifests diff --git a/src/http/context.ts b/src/http/context.ts index ce00524..befd0bf 100644 --- a/src/http/context.ts +++ b/src/http/context.ts @@ -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 // wraps the hrefs it builds itself. localeHref(href: string): string; - // Every installed locale, sorted. With `localeLabel` (from the barrel) it is what a plugin needs - // to build its own language picker; the host's own picker is already in the shell. + // Every installed locale, sorted. With `localeLabel` (from @plainpages/plugin-api) it is what a + // plugin needs to build its own language picker; the host's own picker is already in the shell. locales: string[]; // 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 diff --git a/src/plugin-host/discovery.test.ts b/src/plugin-host/discovery.test.ts index 2b633cf..8bdca5a 100644 --- a/src/plugin-host/discovery.test.ts +++ b/src/plugin-host/discovery.test.ts @@ -61,9 +61,9 @@ const badCases: Array<{ name: string; files: Record; 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 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 }, - // Written by the documented install command with one path segment dropped — and it would silently - // make every plugin below it part of its scope. + // `npm install --prefix plugins` — the documented command with one path segment dropped. { 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 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/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) }) }] });`, - "node_modules/hoisted/index.js": `export default 1;`, }); 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" }); }); -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 dir = scaffold(t, {}); symlinkSync(join(ownRepo, "my-plugin"), join(dir, "linked")); diff --git a/src/plugin-host/discovery.ts b/src/plugin-host/discovery.ts index 1d4953c..fd36dd7 100644 --- a/src/plugin-host/discovery.ts +++ b/src/plugin-host/discovery.ts @@ -27,8 +27,12 @@ export async function discoverPlugins(options: DiscoverOptions = {}): Promise, not plugins/`); + // `npm install --prefix plugins` instead of `--prefix plugins/`: the package.json becomes the + // 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/`); + } } for (const id of pluginFolders(dir)) { @@ -83,9 +87,8 @@ export async function discoverPlugins(options: DiscoverOptions = {}): Promise