Refuse a stray package.json in config/, and let Renovate reach the example plugins #77

Merged
lilleman merged 6 commits from config-guard into main 2026-08-18 22:10:19 +02:00
9 changed files with 40 additions and 12 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": [
+1
View File
@@ -113,6 +113,7 @@ test("the code field guards a pasted space: one-time-code autofill + numeric inp
);
assert.deepEqual(view.fields.find((f) => f.name === "code"), {
autocomplete: "one-time-code", // Kratos sends none for the OTP node — enable OS/email autofill
hint: "Digits only — no spaces.", // the pattern refusal alone reads as a bare "match the requested format"
icon: "i-shield",
id: "field-code",
inputmode: "numeric",
+2 -1
View File
@@ -11,6 +11,7 @@ import type { Flow, FlowType, UiNode } from "./kratos-public.ts";
export interface FlowField {
autocomplete?: string;
error?: { text: string };
hint?: string; // muted helper text under the input
icon?: string; // Lucide sprite id for the input
id: string;
inputmode?: string; // virtual-keyboard hint (e.g. "numeric" for the OTP code)
@@ -139,7 +140,7 @@ function toField(node: UiNode, name: string, type: string, t: Translate): FlowFi
...(autocomplete ? { autocomplete } : {}),
...(errorMsg ? { error: { text: kratosText(t, errorMsg.text, idKey(errorMsg.id)) } } : {}),
...(icon ? { icon } : {}),
...(isCode ? { inputmode: "numeric", pattern: "[0-9]*" } : {}),
...(isCode ? { hint: t("auth.field.code.hint"), inputmode: "numeric", pattern: "[0-9]*" } : {}),
...(node.attributes["required"] === true ? { required: true } : {}),
...(value ? { value } : {}),
};
+1
View File
@@ -9,6 +9,7 @@ const messages = {
"auth.continue": "Continue",
// Kratos labels its own form fields; these translate the ones the built-in identity schema uses,
// keyed on the input name. A deployment's extra traits keep Kratos' label until a plugin covers them.
"auth.field.code.hint": "Digits only — no spaces.",
"auth.field.email": "Email",
"auth.field.identifier": "Email",
"auth.field.password": "Password",
+1
View File
@@ -2,6 +2,7 @@ import type { CoreMessages } from "./en-US.ts";
const messages: CoreMessages = {
"auth.continue": "Fortsätt",
"auth.field.code.hint": "Endast siffror — inga mellanslag.",
"auth.field.email": "E-postadress",
"auth.field.identifier": "E-postadress",
"auth.field.password": "Lösenord",
+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);
+5 -5
View File
@@ -2,22 +2,19 @@
## 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.
- [ ] Record the browser floor Plainpages requires, and whether the fallback is the contract or a courtesy. The stylesheet needs `:has()` (Dec 2023); the menus need the popover API (Safari 17) and CSS anchor positioning for placement (newer still, and unguarded — the `@supports` test covers popover only). An iPadOS 16 tablet, capped at Safari 16, therefore gets panels flowing inline rather than working menus. Either state a supported floor or accept the fallback for those devices; nobody has rendered that path on real hardware.
- [ ] Decide whether the profile dropdown still earns a dropdown. It holds one item, Sign out, behind a click, and its "Signed in as X" head repeats what the trigger already shows.
- [ ] Trim whitespace around the verification code in the form — a copy+pasted code from the email currently fails.
- [ ] Trim whitespace around the verification code — a copy+pasted code from the email fails, today as a browser `pattern` refusal rather than a Kratos rejection, now carrying a digits-only hint so that refusal isn't bare. The paste still fails. **Own session.** No zero-JS fix exists: `pattern` is rejective and cannot transform, so loosening it only lets the untrimmed value through. The real fix is a host-side proxy of the flow POST, which is feasible and half-built — `submitFlow` (`src/auth/kratos-public.ts:132`) already relays cookies and normalises a 422 `redirect_browser_to`, and has no callers outside tests. CSRF improves rather than breaks: the host already writes Kratos' cookie onto its own origin (`src/auth/routes.ts:77`), so the POST becomes same-origin, and `form-action 'self'` could finally join the CSP. Two risks: one `flow.ui.action` serves all five flows, so this rewrites the sign-in path, not just code entry; and Go's nosurf checks Referer only over https, which no test here covers. **Check first:** `kratos-admin.ts:75` returns a `recovery_link` alongside the code — if the stock courier mail carries one (unverified), a template change fixes the UX with no host code. Code entry has no E2E coverage today either way.
- [ ] Guard against the double-clicked submit, without client-side JavaScript. A non-technical user clicks a button twice when nothing happens fast enough, so a second identical POST is an expected event, not misuse — today it creates two users, mints two recovery codes, or registers two OAuth2 clients. Constraints: HTML/CSS only, and it must not break an action that is *legitimately* repeatable. Sketch: a CSS-only affordance so the second click has nothing to hit, paired with the host recognising a duplicate on the server — same session, route and payload within a short window — then logging and dropping it. Open questions: what identifies "the same submission" (a one-time token minted into each rendered form beats hashing the payload, and the CSRF plumbing already mints per-request tokens), the window length, where the record lives given the app is stateless, and how a plugin declares a route repeatable.
- [ ] Decide the caching contract for rendered pages. Responses carry `Vary: Accept-Language` but nothing sets `Cache-Control`, so a shared cache has no instruction and a signed-in page is not marked `private`. Either set the headers deliberately (public cacheable, gated `private, no-store`) or record in AGENTS.md that the reverse proxy owns this.
- [ ] Decide whether the single generic Keto `Resource` namespace should become per-domain namespaces (`Shift`, `Document`, …), as Ory's own examples model it. One global `Resource` bucket is the project's own "no catch-all names" rule applied to namespaces. A design question, not a naming one.
@@ -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.