Follow symlinked plugin folders, and keep a plugin .npmrc out of the image
CI / full-gate (push) Successful in 2m44s

This commit is contained in:
lilleman
2026-08-18 07:49:51 +02:00
parent 77343e859a
commit 3f9787df48
5 changed files with 32 additions and 5 deletions
+4
View File
@@ -6,6 +6,10 @@ 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.
plugins/**/.npmrc
e2e-tests/artifacts
# Orchestration, not test code — keep them out of the runner image (COPY e2e-tests/ ./)
e2e-tests/Dockerfile
+5
View File
@@ -82,6 +82,11 @@ Revisit only if the stated reason stops holding.
- **`config/` is still a plain dir — no `package.json` of its own**, or `#menu-config` resolves
against that instead and boot fails loud. An operator's menu override has no use for
dependencies; if that changes, it needs the same package treatment.
- **A plugin `package.json` without `"type": "module"` is refused, not warned.** Allowing it costs a
warning and a re-parse per file, not a break — Node detects module syntax, so even a `.js` helper
loads — and an operator on a read-only third-party mount cannot apply the remedy. Refused anyway
because the direction is safe: refuse→warn relaxes freely, warn→refuse breaks installed plugins.
**Valid while nothing is installed in the wild.**
- **`examples/` mirrors the drop-in mount dirs** — `examples/plugins/<id>/` copies to
`plugins/<id>/`, `examples/config/menu.ts` to `config/menu.ts`. Both mirrors are in
`tsconfig.include` and resolve the host through the barrels, so each typechecks in place *and*
+18 -1
View File
@@ -1,5 +1,5 @@
import assert from "node:assert/strict";
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
import { mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import { dirname, join } from "node:path";
import { test, type TestContext } from "node:test";
@@ -127,6 +127,23 @@ test("a plugin may carry its own package.json, node_modules and dependencies", a
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) => {
const ownRepo = scaffold(t, { "my-plugin/plugin.ts": full("my-plugin") });
const dir = scaffold(t, {});
symlinkSync(join(ownRepo, "my-plugin"), join(dir, "linked"));
const plugins = await discoverPlugins({ dir });
assert.deepEqual(plugins.map((p) => p.id), ["linked"]); // the link name is the id, not the target's
});
test("a dangling plugin symlink fails loud rather than vanishing", async (t) => {
const dir = scaffold(t, {});
symlinkSync(join(dir, "gone"), join(dir, "broken"));
await assert.rejects(discoverPlugins({ dir }), /broken.*plugin\.ts/s);
});
test("a shared permission name only warns — both plugins still load", async (t) => {
const shared = `export default { apiVersion: "1.0.0", permissions: [{ name: "shared:read" }] };`;
const dir = scaffold(t, { "x/plugin.ts": shared, "y/plugin.ts": shared });
+3 -3
View File
@@ -84,11 +84,11 @@ export async function discoverPlugins(options: DiscoverOptions = {}): Promise<Pl
}
// Subfolders of plugins/, sorted for deterministic load order + stable conflict messages. Hidden
// entries (.git, .DS_Store, …) and non-directories are skipped — only folders are plugins. So is
// node_modules, which npm leaves here when a dependency install is pointed at plugins/ itself.
// entries, plain files and node_modules are skipped; a symlink counts, so a plugin kept in its own
// repo joins the tree with `ln -s`, and a dangling one trips "no plugin.ts found" rather than vanishing.
function pluginFolders(dir: string): string[] {
return readdirSync(dir, { withFileTypes: true })
.filter((e) => e.isDirectory() && !e.name.startsWith(".") && e.name !== "node_modules")
.filter((e) => (e.isDirectory() || e.isSymbolicLink()) && !e.name.startsWith(".") && e.name !== "node_modules")
.map((e) => e.name)
.sort();
}
+2 -1
View File
@@ -3,7 +3,8 @@
## Unfinnished work
- [ ] Add a way to configure plugins directly when installing. Most reasonable is an .env file in the plugin folder, I think, but I am open to suggestions.
- [ ] Decide Renovate's reach over plugin dependencies before the first example plugin takes one. `renovate.json` extends `config:recommended`, whose `:ignoreModulesAndTests` ignores `**/examples/**`, so an example plugin's `package.json` would get no update PRs and nobody would notice. Either narrow the ignorePath or state that plugin deps are the plugin owner's to update (README → Plugin dependencies already says so for external plugins). Raised by the architecture review 2026-08-17.
- [ ] Give `config/` the same named refusal plugin folders now get for a stray `package.json`. It already fails loud, but as `ERR_PACKAGE_IMPORT_NOT_DEFINED` wrapped in a `config/menu.ts failed to import`, naming neither the cause nor the remedy — and now that a plugin folder may legitimately hold a `package.json`, the asymmetry between the two drop-in dirs lives only in prose.
- [ ] Decide Renovate's reach over plugin dependencies before the first example plugin takes one. `renovate.json` extends `config:recommended`, whose `:ignoreModulesAndTests` ignores `**/examples/**`, so an example plugin's `package.json` would get no update PRs and nobody would notice. Either narrow the ignorePath or state that plugin deps are the plugin owner's to update (README → Plugin dependencies already says so for external plugins).
- [ ] Rename the plugin "admin" to something less generic, like "auth-admin" or "users-groups-admin".
- [ ] Guard the group paths against self-lockout, or accept them explicitly. The self-revoke guard covers only your own *direct* grants on the Users screen; unticking a permission on a group you belong to, removing yourself from that group, or deleting it can all still strip your own effective access with no warning. Recorded in AGENTS.md as a known gap — the robust fix is a "last effective holder" check, which needs a reverse Keto query.
- [ ] The permission picker has no concurrency baseline, so two operators editing the same user/group silently discard each other's change. Sketch: post the rendered set as a hidden baseline; if it no longer matches Keto, re-render with "this changed while you had the page open" rather than applying.