Compare commits
3 Commits
f992cb6b2c
...
950eb5a911
| Author | SHA1 | Date | |
|---|---|---|---|
| 950eb5a911 | |||
| 091011cfe5 | |||
| d55898eb8c |
@@ -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.
|
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 every manifest in this
|
||||||
only.
|
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
|
- **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`.
|
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
|
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
|
`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
|
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`
|
(keep the mounted `config/` a plain dir — no `package.json` of its own):
|
||||||
resolves against that instead and boot fails loud):
|
|
||||||
```ts
|
```ts
|
||||||
import { defineMenu } from "#menu-config";
|
import { defineMenu } from "#menu-config";
|
||||||
export default defineMenu({ branding: { name: "Acme Ops" }, override: { hide: ["teams"] } });
|
export default defineMenu({ branding: { name: "Acme Ops" }, override: { hide: ["teams"] } });
|
||||||
|
|||||||
@@ -1,6 +1,8 @@
|
|||||||
{
|
{
|
||||||
"$schema": "https://docs.renovatebot.com/renovate-schema.json",
|
"$schema": "https://docs.renovatebot.com/renovate-schema.json",
|
||||||
"extends": ["config:recommended"],
|
"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,
|
"automerge": true,
|
||||||
"commitBody": "Release-Bump: {{{updateType}}}",
|
"commitBody": "Release-Bump: {{{updateType}}}",
|
||||||
"packageRules": [
|
"packageRules": [
|
||||||
|
|||||||
@@ -1,16 +1,20 @@
|
|||||||
import assert from "node:assert/strict";
|
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 { tmpdir } from "node:os";
|
||||||
import { join } from "node:path";
|
import { join } from "node:path";
|
||||||
import { test, type TestContext } from "node:test";
|
import { test, type TestContext } from "node:test";
|
||||||
import { DEFAULT_MENU, loadMenuConfig } from "./menu-config.ts";
|
import { DEFAULT_MENU, loadMenuConfig } from "./menu-config.ts";
|
||||||
|
|
||||||
// Write a throwaway menu.ts (a plain object — defineMenu is identity) and clean it up after.
|
// 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-"));
|
const dir = mkdtempSync(join(tmpdir(), "pp-menu-"));
|
||||||
t.after(() => rmSync(dir, { force: true, recursive: true }));
|
t.after(() => rmSync(dir, { force: true, recursive: true }));
|
||||||
const file = join(dir, "menu.ts");
|
const file = join(dir, "menu.ts");
|
||||||
writeFileSync(file, source);
|
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;
|
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 { branding: { theme: "neon" } };`) }), /theme/);
|
||||||
await assert.rejects(loadMenuConfig({ file: scaffold(t, `export default { override: { hide: "teams" } };`) }), /hide.*array/s);
|
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"),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|||||||
@@ -50,6 +50,14 @@ export async function loadMenuConfig(options: LoadMenuOptions = {}): Promise<Men
|
|||||||
const file = options.file ?? MENU_CONFIG_FILE;
|
const file = options.file ?? MENU_CONFIG_FILE;
|
||||||
if (!existsSync(file)) return DEFAULT_MENU; // clean clone: no central override
|
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 };
|
let mod: { default?: unknown };
|
||||||
try {
|
try {
|
||||||
mod = await import(pathToFileURL(file).href);
|
mod = await import(pathToFileURL(file).href);
|
||||||
|
|||||||
@@ -2,16 +2,13 @@
|
|||||||
|
|
||||||
## Unfinnished work
|
## 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.
|
- [ ] 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.
|
||||||
- [ ] 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".
|
- [ ] 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.
|
- [ ] 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.
|
- [ ] 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 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.
|
- [ ] 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.
|
- [ ] 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.
|
- [ ] 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 `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.
|
- [ ] 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
|
## 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] 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] 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.
|
- [x] Enforce `<resource>:<action>` permission names at discovery; split `admin` per screen.
|
||||||
|
|||||||
Reference in New Issue
Block a user