From 82af77356f81c8ff1c0f98790ca09d919d61deb0 Mon Sep 17 00:00:00 2001
From: lilleman <lilleman@larvit.se>
Date: Tue, 18 Aug 2026 07:49:51 +0200
Subject: [PATCH] Follow symlinked plugin folders, and keep a plugin .npmrc out
 of the image

---
 .dockerignore                     |  4 ++++
 AGENTS.md                         |  5 +++++
 src/plugin-host/discovery.test.ts | 19 ++++++++++++++++++-
 src/plugin-host/discovery.ts      |  6 +++---
 todo.md                           |  3 ++-
 5 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/.dockerignore b/.dockerignore
index 05a6c89..2872634 100644
--- a/.dockerignore
+++ b/.dockerignore
@@ -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
diff --git a/AGENTS.md b/AGENTS.md
index dbb4d53..7ed9948 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -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*
diff --git a/src/plugin-host/discovery.test.ts b/src/plugin-host/discovery.test.ts
index 0243dc8..2b633cf 100644
--- a/src/plugin-host/discovery.test.ts
+++ b/src/plugin-host/discovery.test.ts
@@ -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 });
diff --git a/src/plugin-host/discovery.ts b/src/plugin-host/discovery.ts
index e69d203..1d4953c 100644
--- a/src/plugin-host/discovery.ts
+++ b/src/plugin-host/discovery.ts
@@ -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();
 }
diff --git a/todo.md b/todo.md
index 2a25397..da4c85f 100644
--- a/todo.md
+++ b/todo.md
@@ -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.