3 Commits

Author SHA1 Message Date
lilleman 950eb5a911 Todo: record the manifest-over-.env decision for plugin config
CI / full-gate (push) Successful in 2m47s
2026-08-18 21:55:35 +02:00
lilleman 091011cfe5 Let Renovate reach the example plugins' manifests 2026-08-18 21:55:35 +02:00
lilleman d55898eb8c Refuse a stray package.json or node_modules in config/ by name 2026-08-18 21:55:32 +02:00
5 changed files with 34 additions and 10 deletions
+3 -4
View File
@@ -713,8 +713,8 @@ included, since the plugin folder *is* the repo. A baked image needs no extra st
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
only.
keeping yours current, and audited, is yours to own. Renovate here watches every manifest in this
repo, the example plugins included — a plugin in its own repo needs its own.
- **Depend on packages that ship JavaScript.** Node refuses to strip types under `node_modules`, so a
dependency whose entry is `.ts` fails at import with `ERR_UNSUPPORTED_NODE_MODULES_TYPE_STRIPPING`.
@@ -759,8 +759,7 @@ The menu is **driven entirely by config** and assembled from two sources:
in or bind-mounting your own dir onto `/app/config` (a commented example sits in
`compose.override.yml`). The file imports its typed builder from **`#menu-config`** (the
subpath import mapped to `src/ui/menu-config.ts`), so it resolves wherever it's mounted
(keep the mounted `config/` a plain dir — no `package.json` of its own — or `#menu-config`
resolves against that instead and boot fails loud):
(keep the mounted `config/` a plain dir — no `package.json` of its own):
```ts
import { defineMenu } from "#menu-config";
export default defineMenu({ branding: { name: "Acme Ops" }, override: { hide: ["teams"] } });
+2
View File
@@ -1,6 +1,8 @@
{
"$schema": "https://docs.renovatebot.com/renovate-schema.json",
"extends": ["config:recommended"],
"description": "ignorePaths overrides config:recommended's :ignoreModulesAndTests, which ignores **/examples/** — an example plugin's dependencies get update PRs like any other manifest here",
"ignorePaths": ["**/node_modules/**"],
"automerge": true,
"commitBody": "Release-Bump: {{{updateType}}}",
"packageRules": [
+17 -2
View File
@@ -1,16 +1,20 @@
import assert from "node:assert/strict";
import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { test, type TestContext } from "node:test";
import { DEFAULT_MENU, loadMenuConfig } from "./menu-config.ts";
// Write a throwaway menu.ts (a plain object — defineMenu is identity) and clean it up after.
function scaffold(t: TestContext, source: string): string {
function scaffold(t: TestContext, source: string, strays: string[] = []): string {
const dir = mkdtempSync(join(tmpdir(), "pp-menu-"));
t.after(() => rmSync(dir, { force: true, recursive: true }));
const file = join(dir, "menu.ts");
writeFileSync(file, source);
for (const stray of strays) {
if (stray.endsWith(".json")) writeFileSync(join(dir, stray), "{}");
else mkdirSync(join(dir, stray), { recursive: true });
}
return file;
}
@@ -37,3 +41,14 @@ test("loadMenuConfig fails loud on a malformed config", async (t) => {
await assert.rejects(loadMenuConfig({ file: scaffold(t, `export default { branding: { theme: "neon" } };`) }), /theme/);
await assert.rejects(loadMenuConfig({ file: scaffold(t, `export default { override: { hide: "teams" } };`) }), /hide.*array/s);
});
test("loadMenuConfig refuses a stray package.json or node_modules beside the config", async (t) => {
const valid = `export default { branding: { name: "Acme Ops" } };`;
for (const stray of ["node_modules", "package.json"]) {
await assert.rejects(
loadMenuConfig({ file: scaffold(t, valid, [stray]) }),
new RegExp(`config/${stray.replace(".", "\\.")} must not exist.*delete`, "s"),
);
}
});
+8
View File
@@ -50,6 +50,14 @@ export async function loadMenuConfig(options: LoadMenuOptions = {}): Promise<Men
const file = options.file ?? MENU_CONFIG_FILE;
if (!existsSync(file)) return DEFAULT_MENU; // clean clone: no central override
// Guarded before the import: Node's own ERR_PACKAGE_IMPORT_NOT_DEFINED names neither cause nor remedy.
const dir = dirname(file);
for (const stray of ["node_modules", "package.json"]) {
if (existsSync(join(dir, stray))) {
throw new Error(`config/${stray} must not exist — it makes config/ its own package scope, so the #menu-config import in config/menu.ts no longer resolves; delete config/{node_modules,package.json,package-lock.json} and keep config/ a plain dir`);
}
}
let mod: { default?: unknown };
try {
mod = await import(pathToFileURL(file).href);
+4 -4
View File
@@ -2,16 +2,13 @@
## 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.
- [ ] 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).
- [ ] Add a way to configure plugins directly when installing. **Decided: the manifest declares it, not an `.env`** — a declared schema is validatable at boot, so a missing or mistyped setting fails loud and named the way a stray `package.json` now does, and the picker/docs can be generated from the declaration. Open: where the operator *supplies* the values (env var per key, a `config/` file, or both), and whether a secret may be declared at all.
- [ ] 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.
- [ ] A grant whose plugin was uninstalled is invisible and unremovable in the GUI. `grantDiff` deliberately never revokes an undeclared name, but nothing *shows* it either — so it can't be audited or cleaned, and reinstalling that plugin silently reactivates access nobody remembers granting. Sketch: a read-only "held, but no installed plugin offers this" list with a remove action.
- [ ] A plugin may gate a route on a permission it never declares — declaring stays optional on purpose. The cost is a dead end: the picker is built from declarations only, so that route is ungrantable from the GUI with no boot error and a permanent 403 as the operator's only clue. Sketch: a discovery *warning* (not an error) naming the gated-but-undeclared permission.
- [ ] Saving permissions gives no confirmation, and a partial failure is silent. `applyGrants` loops writes then deletes with no transaction, so a Keto error midway leaves a half-applied set behind the generic error page; and a successful save is indistinguishable from "nothing changed". The `alert alert-pos` pattern the recovery-code banner uses is already available.
- [ ] The seeded admin@plainpages.local is assigned twice to the same permission; should only be once.
- [ ] In Playwright tests, try different resolutions and sizes, from BIG desktop down to tiny phone.
- [ ] Decide whether `e2e-tests/` should be typechecked. It is outside `tsconfig.include`, so the gate never checks its most logic-bearing file (`console-guard.ts`). Including it needs the DOM lib and `@playwright/test` present wherever `npm run typecheck` runs, which today is the `web` image that installs neither.
- [ ] Decide whether Playwright's `workers` should be pinned. Unset, it sizes the pool from `os.cpus()`, which reports the host's cores regardless of a container CPU quota — and with `retries: 0` a starved runner turns a slow test straight into a red gate. Fine on the current act_runner; revisit if CI ever runs constrained.
@@ -39,6 +36,9 @@ Prioritized. Overall verdict: architecture is sound; these are refinements.
## Finnished work
- [x] Refuse a stray `package.json`/`node_modules` in `config/` by name, as plugin folders already are.
- [x] Let Renovate reach the example plugins' manifests (`ignorePaths` overrides `config:recommended`).
- [x] The seeded admin is granted each permission once — `seedPermissions` dedupes and the grant PUT is idempotent.
- [x] Run the E2E runner as the invoking user so its artifacts aren't root-owned.
- [x] Install node_modules above `WORKDIR /app` so no mount leaves a root-owned dir in the checkout.
- [x] Enforce `<resource>:<action>` permission names at discovery; split `admin` per screen.