From bcf4d7fb1fb71d44c655825c94249b7ec70ab3f4 Mon Sep 17 00:00:00 2001 From: lilleman Date: Wed, 5 Aug 2026 18:18:49 +0200 Subject: [PATCH 1/4] Install deps above WORKDIR so no root-owned node_modules lands in the checkout --- AGENTS.md | 11 +++++++++++ Dockerfile | 15 ++++++++++----- README.md | 14 +++++++++++--- compose.override.yml | 2 -- src/compose.test.ts | 18 +++++++++++++++++- src/ui/icons.test.ts | 3 ++- todo.md | 3 ++- 7 files changed, 53 insertions(+), 13 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index b94f4fb..203072a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -319,6 +319,17 @@ them. Revisit only if the stated reason stops holding. `e2e-tests/playwright.config.ts`). The rest write users, groups and sessions to one shared backend, where a second engine's run would race the first, so widening them means giving each engine its own stack. Screenshots are written per project name for the same reason. Decided 2026-08-05. +- **Deps install to `/node_modules`, one level above `WORKDIR /app`** — Node resolves bare specifiers + upward, so dev's `.:/app` bind mount has nothing to shadow. The usual fix, a volume at + `/app/node_modules`, is what this replaces: the daemon creates a missing mount destination **as + root whatever user the container runs as** (verified — `--user 1000:1000` still yields a root-owned + dir), so it left an empty root-owned `node_modules/` sitting in the developer's own checkout, and + the anonymous volume also went stale on a dep bump until `down -v`. Consequences worth knowing: + nothing may mount inside `/app` at that path again (locked by `src/compose.test.ts`); a path built + as `/node_modules` no longer resolves, so `src/ui/icons.test.ts` locates lucide-static by + specifier via `import.meta.resolve`; and `npm install` must not run in `/app` or it recreates the + problem — README → Extending the core documents `--package-lock-only` plus `--user` instead, which + is also why the image sets `npm_config_cache` (the host uid has no home dir here). Decided 2026-08-05. ## Docker only — no host tooling diff --git a/Dockerfile b/Dockerfile index 4192b48..a05c15a 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,14 +1,19 @@ # Node 24 runs TypeScript directly (type stripping) — no build step. Pinned exact tag. FROM node:24.19.0-alpine3.24 +# Deps land at /node_modules, one level above WORKDIR: Node resolves upward, so dev's `.:/app` +# bind mount cannot shadow them. Mounting a volume at /app/node_modules instead has the daemon +# create that destination in the developer's own checkout, root-owned whatever user runs the +# container. Reproducible install from the lockfile; dev deps kept so typecheck/test run in-image. +COPY package.json package-lock.json .npmrc /deps/ +RUN cd /deps && npm ci && mv node_modules /node_modules && rm -rf /deps + WORKDIR /app - -# Reproducible install from the lockfile. Dev deps kept so typecheck/test run in-image. -COPY package.json package-lock.json .npmrc ./ -RUN npm ci - COPY . . +# Lockfile edits run as the host user (README → Extending the core) so the rewritten files stay +# theirs; that uid has no home in this image, and npm's default cache would land in unwritable /. +ENV npm_config_cache=/tmp/.npm ENV PORT=3000 EXPOSE 3000 CMD ["node", "src/server.ts"] diff --git a/README.md b/README.md index 3024af3..4ed6368 100644 --- a/README.md +++ b/README.md @@ -1964,9 +1964,17 @@ README-dockerhub.md The Docker Hub repository description (docker.io/larvit/pla - **New page in a plugin:** add a route + handler to the plugin manifest and a template in its `views/`. - **Static asset:** drop it in the plugin's `public/`; served at `/public//`. -- **New dependency:** `docker compose run --rm web npm install ` (updates `package.json` - + `package-lock.json`), then `docker compose build`. Keep deps minimal — prefer the Node - standard library, and prefer an Ory REST call over an SDK. +- **New dependency:** update the manifest + lockfile, then rebuild — the image is where deps + live, so there is nothing to install into the checkout: + + ```bash + docker compose run --rm --no-deps --user "$(id -u):$(id -g)" web npm install --package-lock-only + docker compose build + ``` + + `--package-lock-only` writes only `package.json` + `package-lock.json`, and `--user` keeps + both yours. Keep deps minimal — prefer the Node standard library, and prefer an Ory REST call + over an SDK. All versions are pinned to **exact, human-readable semantic versions** (no ranges, no digests): npm deps via `.npmrc` (`save-exact=true`) + the committed lockfile (`npm ci`), and diff --git a/compose.override.yml b/compose.override.yml index 52e3e94..29db789 100644 --- a/compose.override.yml +++ b/compose.override.yml @@ -18,7 +18,6 @@ services: SCHEDULING_UPSTREAM: "http://shifts-upstream:4000" # backs the reference plugin once you copy it into plugins/ volumes: - .:/app - - /app/node_modules # Mount your own menu/branding override into the empty config/ dir (defaults apply otherwise): # - ./config:/app/config:ro # your config/menu.ts — see examples/config/menu.ts for a template @@ -32,7 +31,6 @@ services: bootstrap: volumes: - .:/app - - /app/node_modules # Mock backend ready for the reference plugin (examples/plugins/scheduling): plugins/ ships empty, so # the plugin is opt-in — `cp -r examples/plugins/scheduling plugins/scheduling`, restart, and this diff --git a/src/compose.test.ts b/src/compose.test.ts index 8ae5d54..fe53874 100644 --- a/src/compose.test.ts +++ b/src/compose.test.ts @@ -6,7 +6,7 @@ // by running the stack; this catches edits. import { test } from "node:test"; import assert from "node:assert/strict"; -import { readFileSync } from "node:fs"; +import { readFileSync, readdirSync } from "node:fs"; const read = (p: string) => readFileSync(new URL(`../${p}`, import.meta.url), "utf8"); const compose = read("compose.yml"); @@ -89,6 +89,22 @@ test("a one-shot bootstrap seeds the stack before web starts", () => { "web waits for bootstrap to finish"); }); +test("deps live above WORKDIR, so no mount creates a root-owned dir in the checkout", () => { + // The daemon creates a missing mount destination as root whatever user the container runs as, so + // a volume at /app/node_modules leaves a root-owned node_modules/ in the developer's own checkout + // (dev bind-mounts `.:/app`). Installing above /app lets Node resolve upward instead — nothing to + // shadow, so nothing to mount over. + const beforeWorkdir = read("Dockerfile").split("WORKDIR /app")[0]!; + assert.match(beforeWorkdir, /npm ci/, "npm ci runs before WORKDIR /app"); + assert.match(beforeWorkdir, /mv\s+node_modules\s+\/node_modules/, "and its tree lands at /node_modules"); + + const composeFiles = readdirSync(new URL("../e2e-tests", import.meta.url)) + .filter((f) => f.startsWith("compose.")) + .map((f) => `e2e-tests/${f}`); + for (const f of ["compose.yml", "compose.override.yml", ...composeFiles]) + assert.ok(!read(f).includes("/app/node_modules"), `${f} mounts nothing at /app/node_modules`); +}); + test("the visual E2E does not drag in the Ory stack", () => { // web's Ory deps are reset for E2E (the dashboard is mock data — no Ory needed). assert.match(visual, /depends_on:\s*!reset\b/, "E2E resets web's depends_on"); diff --git a/src/ui/icons.test.ts b/src/ui/icons.test.ts index 0587733..7c91961 100644 --- a/src/ui/icons.test.ts +++ b/src/ui/icons.test.ts @@ -7,7 +7,8 @@ import ejs from "ejs"; import { ICON_NAMES, buildIconSprite } from "./icons.ts"; const rootDir = join(dirname(fileURLToPath(import.meta.url)), "..", ".."); -const lucideDir = join(rootDir, "node_modules", "lucide-static", "icons"); +// Resolved by specifier, not by path: the install lives above the app dir, not in it (Dockerfile). +const lucideDir = join(dirname(fileURLToPath(import.meta.resolve("lucide-static/package.json"))), "icons"); const partial = join(rootDir, "views", "partials", "icons.ejs"); const symbolInner = (sprite: string, id: string): string => diff --git a/todo.md b/todo.md index e26c6b4..ae7ae9f 100644 --- a/todo.md +++ b/todo.md @@ -2,7 +2,7 @@ ## Unfinnished work -- [ ] node_modules gets installed straight to the root folder with root permissions, it should at the very least be same owner as the one running the docker process, or built inside the docker image. +- [ ] `e2e-tests/artifacts/` is written root-owned into the checkout — same complaint as the node_modules one, different mechanism, so it was left out of that fix rather than bundled in. The Playwright container runs as root and the five e2e compose files bind `./e2e-tests/artifacts` in, so screenshots, traces and the HTML report land as `root:root` and need `sudo` to delete. Unlike the mountpoint case a container `user:` *would* fix this (the daemon only forces root on destinations it has to create), but compose has no `$UID` of its own — it needs an `.env` or `id -u` plumbed through `ci.sh` — and the CI artifact upload reads that dir, so it wants checking on the runner rather than only locally. Found while fixing the node_modules item 2026-08-05. - [ ] 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. - [ ] Rename the plugin "admin" to something less generic, like "auth-admin" or "users-groups-admin". - [ ] Guard the group paths to 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. Same scope the deleted Permissions screen had, and recorded in AGENTS.md as a known gap — the robust fix is a "last effective holder" check, which needs a reverse Keto query. Raised by the stability review 2026-08-05. @@ -38,6 +38,7 @@ Prioritized. Overall verdict: architecture is sound (contract-first plugin API, ## Finnished work +- [x] node_modules gets installed straight to the root folder with root permissions, it should at the very least be same owner as the one running the docker process, or built inside the docker image. (It *was* built inside the image — what landed in the checkout was an empty root-owned `node_modules/`, created by the daemon as the mountpoint for `compose.override.yml`'s `- /app/node_modules` anonymous volume, which exists only to stop the `.:/app` bind mount shadowing the image's deps. "Same owner as the one running the docker process" turned out not to be reachable: the daemon creates a missing mount destination as **root regardless of the container user** — measured, `--user 1000:1000` still yields a root-owned dir — so the volume had to go, not be re-owned. Deps now install to `/node_modules`, one level above `WORKDIR /app`; Node resolves bare specifiers upward, so there is nothing for the bind mount to shadow and no volume to mount. Drops both anonymous volumes and their stale-on-dep-bump footgun with them. Three consequences: `src/ui/icons.test.ts` located lucide-static as `/node_modules/...` and now resolves it by specifier via `import.meta.resolve`; the documented `npm install ` would have recreated a root-owned dir, so README → Extending the core now runs it `--package-lock-only --user "$(id -u):$(id -g)"` — writing only the two manifest files, as yours — and the image sets `npm_config_cache=/tmp/.npm` because that uid has no home dir in it; and `src/compose.test.ts` locks the invariant, since re-adding `- /app/node_modules` to fix a resolution problem would silently bring the root-owned dir back. Verified on a live stack: `docker compose up -d --build` boots, bootstrap seeds, `/` serves 200, and the checkout stays clean.) - [x] Document permissions format so it is folled going forward: :, for example scheduling:read. Permission "admin" does not match this, and needs to be users:read, users:write, groups:read, groups:write. (README → [Naming a permission](README.md#naming-a-permission) is the one home for the rule, and the host *enforces* it at discovery — `isValidPermissionName` in `src/plugin-host/plugin.ts`, checked by `shapeError` over every route/nav `permission` and every declared name — so a badly-named permission stops the boot like any other bad manifest, for every plugin rather than only ones the admin GUI touches. `admin` is gone, split per screen into `users:`, `groups:` and `oauth2-clients:` × `read`/`write`. The read/write split is real, not cosmetic: `users:read` opens the list and is refused on every POST, and the Admin nav header lost its own gate so each screen is filtered by its own `:read` — hold none of the three and `composeNav` drops the emptied header (which needs the header to carry no `href`, now asserted). Two things had to be fixed to get here. The permission path validator was the *group* regex with no colon, so `/admin/permissions/scheduling:read` already 404'd. And `ADMIN_PERMISSIONS` defaulting to empty exposed that `bootstrap` never bind-mounted `plugins/` at all — it discovered only the image's empty copy, so a dropped-in plugin's permissions were never seeded; the mount lives in `compose.override.yml` (dev-only, mirroring `web`'s `.:/app`) because the base file gives both services the same baked copy and a base-file mount would collide with the e2e stacks that bind plugins *inside* that path. Quick start now says `docker compose up -d`, which re-runs the one-shot. Verified end to end on a live stack.) - [x] Permissions should be a list in code. Since no permissions exists in the database out of the box, but there are a fixed number of permissions in the plugins that the end consumer and user of plain pages can use, these permissions must surface to the UI somehow. The effects is that the permissions page should be deleted completely, and the users and groups pages should gain the functionality to add permissions to their things instead, provided the user have the right permissiosn to do so, of course. Run the product reviewer agent on this todo also. (The host collects every installed plugin's declarations into one catalog — `declaredPermissions()` → `ctx.declaredPermissions`, deduped and sorted, computed once at wiring — and that catalog *is* the fixed list. The Permissions screen is deleted outright: its module, tests, three views, two partials and 29 catalog keys per locale. Users and Groups each gained a checkbox list of the catalog, ticked where held; the whole set posts back, so what is submitted is the desired state and `grantDiff` turns it into grants + revokes. Two properties earn their tests: a crafted POST cannot grant a name no plugin declares, and a held-but-undeclared name — left over from an uninstalled plugin — is never silently revoked by an unrelated save, since the picker only speaks for what it showed. A user's own change revokes their live tokens; a group's reaches members at their next re-mint, the documented transitive lag. Keto stays optional on the Users screen: without it the page still lists and edits, minus the picker. Maintainer's call 2026-08-05 to keep the OAuth2-clients screen and gate it `oauth2-clients:read/write` — permissions and OAuth2 are orthogonal, scopes say what an *app* may see and permissions what a *user* may do, so the screen only ever needed *a* gate.) - [x] In Playwright tests, check for warnings and errors in all browsers on all the steps. If they exist, that is a failure we need to fix. (Every spec takes its `test` from `e2e-tests/console-guard.ts`, which watches every page a test opens — `console.error`, `console.warning`, and uncaught page errors — and fails the test that provoked one, at whatever step. The bar is zero rather than a curated tolerance list: the app ships no client JavaScript, so a message means a broken sub-resource, a rejected attribute, or an engine refusing a feature. Two narrow escapes, both explicit: the COOP header Chromium drops because the e2e stacks serve plain http over container hostnames (a deployment serves https, where it applies), and per-test `allowConsole(/…/)` — used once, by the 404 spec, whose own navigation Chromium and WebKit log. **All browsers** is now literal for the Ory-free suites: `visual.spec.ts` + `language.spec.ts` run in Chromium, Firefox *and* WebKit — the per-test `@engines` tag is gone, and screenshots are written per project so the three don't fight over one file — which is what makes an engine-specific message visible at all. The Ory-backed suites write users, groups and sessions to one shared backend, so they stay on Chromium; widening them needs a stack per engine. Nothing in the app had to be fixed: the sweep found only the two above. Verified by negative control — an injected `console.warn` failed the test in all three engines and an injected `console.error` failed on full-flow's shared serial page — which also caught the guard registering that page twice. `src/e2e-console-guard.test.ts` locks the wiring in the *unit* gate, since a spec importing `test` straight from Playwright would run unwatched and green.) -- 2.52.0 From 3d3313c0ee99f308e19766444535db3cc20b109e Mon Sep 17 00:00:00 2001 From: lilleman Date: Wed, 5 Aug 2026 19:17:07 +0200 Subject: [PATCH 2/4] Document the shadowing risk and the leftover dir; close two holes in the mount guard --- .dockerignore | 2 ++ AGENTS.md | 7 ++++++- README.md | 7 +++++++ src/compose.test.ts | 15 ++++++++++----- 4 files changed, 25 insertions(+), 6 deletions(-) diff --git a/.dockerignore b/.dockerignore index b5ac416..196eef9 100644 --- a/.dockerignore +++ b/.dockerignore @@ -1,4 +1,6 @@ .git +# Load-bearing: deps live at /node_modules, so a builder's stray copy would bake in at +# /app/node_modules and shadow them for every consumer of the image, production included. node_modules npm-debug.log *.log diff --git a/AGENTS.md b/AGENTS.md index 203072a..7dea587 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -329,7 +329,12 @@ them. Revisit only if the stated reason stops holding. as `/node_modules` no longer resolves, so `src/ui/icons.test.ts` locates lucide-static by specifier via `import.meta.resolve`; and `npm install` must not run in `/app` or it recreates the problem — README → Extending the core documents `--package-lock-only` plus `--user` instead, which - is also why the image sets `npm_config_cache` (the host uid has no home dir here). Decided 2026-08-05. + is also why the image sets `npm_config_cache` (the host uid has no home dir here). Nothing masks + that path any more, so anything at `/app/node_modules` now **shadows `/node_modules` silently** — + wrong dependency code, no warning, and CI stays green because a fresh clone has none. An empty + leftover dir is harmless (resolution falls through); a populated one from the superseded root + `npm install` needs `sudo` to remove, which is worse than the bug this replaced. `.dockerignore`'s + `node_modules` line is what keeps a builder's stray copy out of the image. Decided 2026-08-05. ## Docker only — no host tooling diff --git a/README.md b/README.md index 4ed6368..a64dd5d 100644 --- a/README.md +++ b/README.md @@ -1768,6 +1768,13 @@ so for now the error names the rule it tripped instead. `users:`/`groups:`/`oauth2-clients:` × `read`/`write`, so a copy taken before this needs re-copying. `ADMIN_PERMISSIONS` is held to the same rule, but an unusable value there is dropped with a warning rather than failing the boot. See [Naming a permission](#naming-a-permission). +- **Deps moved to `/node_modules`, above `/app`** (2026-08-05). The dev override no longer mounts an + anonymous volume at `/app/node_modules`, so after pulling, delete the empty directory the daemon + left behind: `rmdir node_modules` — no `sudo`, it is empty. `docker volume prune` reclaims the + orphaned volumes. Keep that path clear: anything there shadows the image's deps silently, and a + populated one is root-owned and needs `sudo` to remove. Add a dependency with the + `--package-lock-only` command in [Extending the core](#extending-the-core), never a bare + `npm install` in `/app`. ## Observability diff --git a/src/compose.test.ts b/src/compose.test.ts index fe53874..535c2a8 100644 --- a/src/compose.test.ts +++ b/src/compose.test.ts @@ -94,14 +94,19 @@ test("deps live above WORKDIR, so no mount creates a root-owned dir in the check // a volume at /app/node_modules leaves a root-owned node_modules/ in the developer's own checkout // (dev bind-mounts `.:/app`). Installing above /app lets Node resolve upward instead — nothing to // shadow, so nothing to mount over. - const beforeWorkdir = read("Dockerfile").split("WORKDIR /app")[0]!; + const dockerfile = read("Dockerfile"); + // Asserted, not assumed: split() returns the whole file when the marker is missing, which would + // silently widen "before WORKDIR" to "anywhere". + assert.ok(dockerfile.includes("WORKDIR /app"), "the app dir is /app"); + const beforeWorkdir = dockerfile.split("WORKDIR /app")[0]!; assert.match(beforeWorkdir, /npm ci/, "npm ci runs before WORKDIR /app"); assert.match(beforeWorkdir, /mv\s+node_modules\s+\/node_modules/, "and its tree lands at /node_modules"); - const composeFiles = readdirSync(new URL("../e2e-tests", import.meta.url)) - .filter((f) => f.startsWith("compose.")) - .map((f) => `e2e-tests/${f}`); - for (const f of ["compose.yml", "compose.override.yml", ...composeFiles]) + const composeFiles = (dir: string) => + readdirSync(new URL(`../${dir}`, import.meta.url)) + .filter((f) => f.startsWith("compose.") && f.endsWith(".yml")) + .map((f) => `${dir}${f}`); + for (const f of [...composeFiles(""), ...composeFiles("e2e-tests/")]) assert.ok(!read(f).includes("/app/node_modules"), `${f} mounts nothing at /app/node_modules`); }); -- 2.52.0 From ab5c24deb7e23cccb26aea3c4d8490fcfce8ee3f Mon Sep 17 00:00:00 2001 From: lilleman Date: Wed, 5 Aug 2026 19:52:09 +0200 Subject: [PATCH 3/4] Cut the node_modules prose to one home each; drop a stray tracked file --- .dockerignore | 3 +-- AGENTS.md | 21 +++++---------------- Dockerfile | 10 ++++------ README.md | 19 ++++++------------- json | 1 - src/compose.test.ts | 8 ++------ todo.md | 4 ++-- 7 files changed, 20 insertions(+), 46 deletions(-) delete mode 100644 json diff --git a/.dockerignore b/.dockerignore index 196eef9..b3c82df 100644 --- a/.dockerignore +++ b/.dockerignore @@ -1,6 +1,5 @@ .git -# Load-bearing: deps live at /node_modules, so a builder's stray copy would bake in at -# /app/node_modules and shadow them for every consumer of the image, production included. +# Load-bearing: a stray copy would bake in at /app/node_modules and shadow /node_modules. node_modules npm-debug.log *.log diff --git a/AGENTS.md b/AGENTS.md index 7dea587..30cd75a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -319,22 +319,11 @@ them. Revisit only if the stated reason stops holding. `e2e-tests/playwright.config.ts`). The rest write users, groups and sessions to one shared backend, where a second engine's run would race the first, so widening them means giving each engine its own stack. Screenshots are written per project name for the same reason. Decided 2026-08-05. -- **Deps install to `/node_modules`, one level above `WORKDIR /app`** — Node resolves bare specifiers - upward, so dev's `.:/app` bind mount has nothing to shadow. The usual fix, a volume at - `/app/node_modules`, is what this replaces: the daemon creates a missing mount destination **as - root whatever user the container runs as** (verified — `--user 1000:1000` still yields a root-owned - dir), so it left an empty root-owned `node_modules/` sitting in the developer's own checkout, and - the anonymous volume also went stale on a dep bump until `down -v`. Consequences worth knowing: - nothing may mount inside `/app` at that path again (locked by `src/compose.test.ts`); a path built - as `/node_modules` no longer resolves, so `src/ui/icons.test.ts` locates lucide-static by - specifier via `import.meta.resolve`; and `npm install` must not run in `/app` or it recreates the - problem — README → Extending the core documents `--package-lock-only` plus `--user` instead, which - is also why the image sets `npm_config_cache` (the host uid has no home dir here). Nothing masks - that path any more, so anything at `/app/node_modules` now **shadows `/node_modules` silently** — - wrong dependency code, no warning, and CI stays green because a fresh clone has none. An empty - leftover dir is harmless (resolution falls through); a populated one from the superseded root - `npm install` needs `sudo` to remove, which is worse than the bug this replaced. `.dockerignore`'s - `node_modules` line is what keeps a builder's stray copy out of the image. Decided 2026-08-05. +- **Deps install to `/node_modules`, above `WORKDIR /app`** — Node resolves upward, so dev's `.:/app` + bind mount has nothing to shadow. Not a volume at `/app/node_modules`: the daemon creates a mount + destination as root whatever `--user` says, leaving a root-owned dir in the checkout. Nothing may + sit at that path now — it shadows `/node_modules` silently (`src/compose.test.ts` guards the compose + files, `.dockerignore` the image). Decided 2026-08-05. ## Docker only — no host tooling diff --git a/Dockerfile b/Dockerfile index a05c15a..f077b3f 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,18 +1,16 @@ # Node 24 runs TypeScript directly (type stripping) — no build step. Pinned exact tag. FROM node:24.19.0-alpine3.24 -# Deps land at /node_modules, one level above WORKDIR: Node resolves upward, so dev's `.:/app` -# bind mount cannot shadow them. Mounting a volume at /app/node_modules instead has the daemon -# create that destination in the developer's own checkout, root-owned whatever user runs the -# container. Reproducible install from the lockfile; dev deps kept so typecheck/test run in-image. +# Above WORKDIR so dev's `.:/app` bind mount can't shadow them; a volume at /app/node_modules +# instead leaves a root-owned dir in the checkout (the daemon creates mount destinations as root). +# Dev deps kept so typecheck/test run in-image. COPY package.json package-lock.json .npmrc /deps/ RUN cd /deps && npm ci && mv node_modules /node_modules && rm -rf /deps WORKDIR /app COPY . . -# Lockfile edits run as the host user (README → Extending the core) so the rewritten files stay -# theirs; that uid has no home in this image, and npm's default cache would land in unwritable /. +# The host uid running a lockfile edit has no home here, so npm's cache would land in unwritable /. ENV npm_config_cache=/tmp/.npm ENV PORT=3000 EXPOSE 3000 diff --git a/README.md b/README.md index a64dd5d..682e805 100644 --- a/README.md +++ b/README.md @@ -1768,13 +1768,9 @@ so for now the error names the rule it tripped instead. `users:`/`groups:`/`oauth2-clients:` × `read`/`write`, so a copy taken before this needs re-copying. `ADMIN_PERMISSIONS` is held to the same rule, but an unusable value there is dropped with a warning rather than failing the boot. See [Naming a permission](#naming-a-permission). -- **Deps moved to `/node_modules`, above `/app`** (2026-08-05). The dev override no longer mounts an - anonymous volume at `/app/node_modules`, so after pulling, delete the empty directory the daemon - left behind: `rmdir node_modules` — no `sudo`, it is empty. `docker volume prune` reclaims the - orphaned volumes. Keep that path clear: anything there shadows the image's deps silently, and a - populated one is root-owned and needs `sudo` to remove. Add a dependency with the - `--package-lock-only` command in [Extending the core](#extending-the-core), never a bare - `npm install` in `/app`. +- **Deps moved to `/node_modules`, above `/app`** (2026-08-05). Run `rmdir node_modules` after + pulling (it is empty — no `sudo`) and `docker volume prune`. Keep that path clear: anything there + silently shadows the image's deps. ## Observability @@ -1971,18 +1967,15 @@ README-dockerhub.md The Docker Hub repository description (docker.io/larvit/pla - **New page in a plugin:** add a route + handler to the plugin manifest and a template in its `views/`. - **Static asset:** drop it in the plugin's `public/`; served at `/public//`. -- **New dependency:** update the manifest + lockfile, then rebuild — the image is where deps - live, so there is nothing to install into the checkout: +- **New dependency:** deps live in the image, so update the manifest + lockfile and rebuild — + `--package-lock-only` writes nothing into the checkout, `--user` keeps the two files yours. + Keep deps minimal — prefer the Node standard library, and an Ory REST call over an SDK. ```bash docker compose run --rm --no-deps --user "$(id -u):$(id -g)" web npm install --package-lock-only docker compose build ``` - `--package-lock-only` writes only `package.json` + `package-lock.json`, and `--user` keeps - both yours. Keep deps minimal — prefer the Node standard library, and prefer an Ory REST call - over an SDK. - All versions are pinned to **exact, human-readable semantic versions** (no ranges, no digests): npm deps via `.npmrc` (`save-exact=true`) + the committed lockfile (`npm ci`), and container images by tag in the `Dockerfile` / compose files (e.g. `node:24.16.0-alpine3.24`, diff --git a/json b/json deleted file mode 100644 index 7dd567a..0000000 --- a/json +++ /dev/null @@ -1 +0,0 @@ -{"errors":null,"message":"not found","url":"https://gitea.larvit.se/api/swagger"} \ No newline at end of file diff --git a/src/compose.test.ts b/src/compose.test.ts index 535c2a8..340f014 100644 --- a/src/compose.test.ts +++ b/src/compose.test.ts @@ -90,13 +90,9 @@ test("a one-shot bootstrap seeds the stack before web starts", () => { }); test("deps live above WORKDIR, so no mount creates a root-owned dir in the checkout", () => { - // The daemon creates a missing mount destination as root whatever user the container runs as, so - // a volume at /app/node_modules leaves a root-owned node_modules/ in the developer's own checkout - // (dev bind-mounts `.:/app`). Installing above /app lets Node resolve upward instead — nothing to - // shadow, so nothing to mount over. + // A volume at /app/node_modules would leave a root-owned dir in the checkout (AGENTS.md). const dockerfile = read("Dockerfile"); - // Asserted, not assumed: split() returns the whole file when the marker is missing, which would - // silently widen "before WORKDIR" to "anywhere". + // split() returns the whole file when the marker is missing, widening "before" to "anywhere". assert.ok(dockerfile.includes("WORKDIR /app"), "the app dir is /app"); const beforeWorkdir = dockerfile.split("WORKDIR /app")[0]!; assert.match(beforeWorkdir, /npm ci/, "npm ci runs before WORKDIR /app"); diff --git a/todo.md b/todo.md index ae7ae9f..737be68 100644 --- a/todo.md +++ b/todo.md @@ -2,7 +2,7 @@ ## Unfinnished work -- [ ] `e2e-tests/artifacts/` is written root-owned into the checkout — same complaint as the node_modules one, different mechanism, so it was left out of that fix rather than bundled in. The Playwright container runs as root and the five e2e compose files bind `./e2e-tests/artifacts` in, so screenshots, traces and the HTML report land as `root:root` and need `sudo` to delete. Unlike the mountpoint case a container `user:` *would* fix this (the daemon only forces root on destinations it has to create), but compose has no `$UID` of its own — it needs an `.env` or `id -u` plumbed through `ci.sh` — and the CI artifact upload reads that dir, so it wants checking on the runner rather than only locally. Found while fixing the node_modules item 2026-08-05. +- [ ] `e2e-tests/artifacts/` is written `root:root` into the checkout and needs `sudo` to delete — the Playwright container runs as root. Unlike the node_modules mountpoint, a container `user:` would fix it, but compose has no `$UID` of its own (needs an `.env` or `id -u` via `ci.sh`) and CI's artifact upload reads that dir. Found 2026-08-05. - [ ] 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. - [ ] Rename the plugin "admin" to something less generic, like "auth-admin" or "users-groups-admin". - [ ] Guard the group paths to 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. Same scope the deleted Permissions screen had, and recorded in AGENTS.md as a known gap — the robust fix is a "last effective holder" check, which needs a reverse Keto query. Raised by the stability review 2026-08-05. @@ -38,7 +38,7 @@ Prioritized. Overall verdict: architecture is sound (contract-first plugin API, ## Finnished work -- [x] node_modules gets installed straight to the root folder with root permissions, it should at the very least be same owner as the one running the docker process, or built inside the docker image. (It *was* built inside the image — what landed in the checkout was an empty root-owned `node_modules/`, created by the daemon as the mountpoint for `compose.override.yml`'s `- /app/node_modules` anonymous volume, which exists only to stop the `.:/app` bind mount shadowing the image's deps. "Same owner as the one running the docker process" turned out not to be reachable: the daemon creates a missing mount destination as **root regardless of the container user** — measured, `--user 1000:1000` still yields a root-owned dir — so the volume had to go, not be re-owned. Deps now install to `/node_modules`, one level above `WORKDIR /app`; Node resolves bare specifiers upward, so there is nothing for the bind mount to shadow and no volume to mount. Drops both anonymous volumes and their stale-on-dep-bump footgun with them. Three consequences: `src/ui/icons.test.ts` located lucide-static as `/node_modules/...` and now resolves it by specifier via `import.meta.resolve`; the documented `npm install ` would have recreated a root-owned dir, so README → Extending the core now runs it `--package-lock-only --user "$(id -u):$(id -g)"` — writing only the two manifest files, as yours — and the image sets `npm_config_cache=/tmp/.npm` because that uid has no home dir in it; and `src/compose.test.ts` locks the invariant, since re-adding `- /app/node_modules` to fix a resolution problem would silently bring the root-owned dir back. Verified on a live stack: `docker compose up -d --build` boots, bootstrap seeds, `/` serves 200, and the checkout stays clean.) +- [x] node_modules gets installed straight to the root folder with root permissions, it should at the very least be same owner as the one running the docker process, or built inside the docker image. (It *was* built in the image; the checkout got an empty root-owned dir — the mountpoint for `compose.override.yml`'s `- /app/node_modules` volume. Re-owning it is impossible (the daemon creates mount destinations as root whatever `--user` says), so deps moved to `/node_modules` above `WORKDIR /app` and the volume is gone. See AGENTS.md.) - [x] Document permissions format so it is folled going forward: :, for example scheduling:read. Permission "admin" does not match this, and needs to be users:read, users:write, groups:read, groups:write. (README → [Naming a permission](README.md#naming-a-permission) is the one home for the rule, and the host *enforces* it at discovery — `isValidPermissionName` in `src/plugin-host/plugin.ts`, checked by `shapeError` over every route/nav `permission` and every declared name — so a badly-named permission stops the boot like any other bad manifest, for every plugin rather than only ones the admin GUI touches. `admin` is gone, split per screen into `users:`, `groups:` and `oauth2-clients:` × `read`/`write`. The read/write split is real, not cosmetic: `users:read` opens the list and is refused on every POST, and the Admin nav header lost its own gate so each screen is filtered by its own `:read` — hold none of the three and `composeNav` drops the emptied header (which needs the header to carry no `href`, now asserted). Two things had to be fixed to get here. The permission path validator was the *group* regex with no colon, so `/admin/permissions/scheduling:read` already 404'd. And `ADMIN_PERMISSIONS` defaulting to empty exposed that `bootstrap` never bind-mounted `plugins/` at all — it discovered only the image's empty copy, so a dropped-in plugin's permissions were never seeded; the mount lives in `compose.override.yml` (dev-only, mirroring `web`'s `.:/app`) because the base file gives both services the same baked copy and a base-file mount would collide with the e2e stacks that bind plugins *inside* that path. Quick start now says `docker compose up -d`, which re-runs the one-shot. Verified end to end on a live stack.) - [x] Permissions should be a list in code. Since no permissions exists in the database out of the box, but there are a fixed number of permissions in the plugins that the end consumer and user of plain pages can use, these permissions must surface to the UI somehow. The effects is that the permissions page should be deleted completely, and the users and groups pages should gain the functionality to add permissions to their things instead, provided the user have the right permissiosn to do so, of course. Run the product reviewer agent on this todo also. (The host collects every installed plugin's declarations into one catalog — `declaredPermissions()` → `ctx.declaredPermissions`, deduped and sorted, computed once at wiring — and that catalog *is* the fixed list. The Permissions screen is deleted outright: its module, tests, three views, two partials and 29 catalog keys per locale. Users and Groups each gained a checkbox list of the catalog, ticked where held; the whole set posts back, so what is submitted is the desired state and `grantDiff` turns it into grants + revokes. Two properties earn their tests: a crafted POST cannot grant a name no plugin declares, and a held-but-undeclared name — left over from an uninstalled plugin — is never silently revoked by an unrelated save, since the picker only speaks for what it showed. A user's own change revokes their live tokens; a group's reaches members at their next re-mint, the documented transitive lag. Keto stays optional on the Users screen: without it the page still lists and edits, minus the picker. Maintainer's call 2026-08-05 to keep the OAuth2-clients screen and gate it `oauth2-clients:read/write` — permissions and OAuth2 are orthogonal, scopes say what an *app* may see and permissions what a *user* may do, so the screen only ever needed *a* gate.) - [x] In Playwright tests, check for warnings and errors in all browsers on all the steps. If they exist, that is a failure we need to fix. (Every spec takes its `test` from `e2e-tests/console-guard.ts`, which watches every page a test opens — `console.error`, `console.warning`, and uncaught page errors — and fails the test that provoked one, at whatever step. The bar is zero rather than a curated tolerance list: the app ships no client JavaScript, so a message means a broken sub-resource, a rejected attribute, or an engine refusing a feature. Two narrow escapes, both explicit: the COOP header Chromium drops because the e2e stacks serve plain http over container hostnames (a deployment serves https, where it applies), and per-test `allowConsole(/…/)` — used once, by the 404 spec, whose own navigation Chromium and WebKit log. **All browsers** is now literal for the Ory-free suites: `visual.spec.ts` + `language.spec.ts` run in Chromium, Firefox *and* WebKit — the per-test `@engines` tag is gone, and screenshots are written per project so the three don't fight over one file — which is what makes an engine-specific message visible at all. The Ory-backed suites write users, groups and sessions to one shared backend, so they stay on Chromium; widening them needs a stack per engine. Nothing in the app had to be fixed: the sweep found only the two above. Verified by negative control — an injected `console.warn` failed the test in all three engines and an injected `console.error` failed on full-flow's shared serial page — which also caught the guard registering that page twice. `src/e2e-console-guard.test.ts` locks the wiring in the *unit* gate, since a spec importing `test` straight from Playwright would run unwatched and green.) -- 2.52.0 From e8b91ecd09a4e180040c9138ba31826afcb31ed5 Mon Sep 17 00:00:00 2001 From: lilleman Date: Wed, 5 Aug 2026 21:52:56 +0200 Subject: [PATCH 4/4] Compress AGENTS.md and add a standing rule to trim it on every edit --- AGENTS.md | 675 ++++++++++++++++++++++++------------------------------ 1 file changed, 296 insertions(+), 379 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 30cd75a..c2a6176 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -3,327 +3,270 @@ Guidance for AI agents and contributors working in this repo. Read `README.md` for commands and layout. +## Maintaining this file + +Every agent session reads this file in full, so its length is a cost paid on every task. +Keep it the shortest thing that still changes what someone does. + +- **Trim as you add.** After any edit, re-read the whole file and compress: merge overlapping + entries, cut prose that restates a rule, drop what the code or `README.md` already says. + Question each section — same information, fewer words. +- **Record the decision and the reason it turns on, nothing else.** Not the investigation, not + what was tried first, not how it was verified — that belongs in the PR that made the change. +- **Give every accepted risk an expiry** ("valid while X"), and delete the entry once X stops + holding. +- **One home per fact.** Link to it rather than restating it — the same sentence in five files + is five things to update and five chances to drift. + ## How to work with tasks Use the file `todo.md`. -For each todo item, interview the user extensively to deeply understand the scope and goal of each. When done, check the completed task in `todo.md`. Commit all changes and push to a new branch, create a PR and merge it when the CI/CD turns green. +For each todo item, interview the user extensively to deeply understand the scope and goal of +each. When done, check the completed task in `todo.md`. Commit all changes and push to a new +branch, create a PR and merge it when the CI/CD turns green. ## Project priorities (do not erode) 1. **Simplicity** — prefer the solution that is easiest to understand, smallest, and most readable. -2. **Few dependencies** — runtime deps stay minimal (today `ejs`, `lucide-static`, - `@larvit/log` — the last itself zero-dependency, for structured/OTLP logging). - Prefer the Node standard library; justify any new dependency; do not add - frameworks. The app is - **stateless — no database**. Auth/identity/OAuth are **Ory sidecar services** - (Kratos/Keto/Hydra, backed by Postgres), reached over their REST APIs with - built-in `fetch` — no SDK dependency. New capabilities ship as **plugin - folders** under `plugins/` that fetch their data from upstream services, not as - core code. See `README.md` for the architecture. +2. **Few dependencies** — runtime deps stay minimal (today `ejs`, `lucide-static`, `@larvit/log`). + Prefer the Node standard library; justify any new dependency; do not add frameworks. The app is + **stateless — no database**. Auth/identity/OAuth are **Ory sidecar services** (Kratos/Keto/Hydra, + backed by Postgres), reached over their REST APIs with built-in `fetch` — no SDK. New + capabilities ship as **plugin folders** under `plugins/` that fetch their data from upstream + services, not as core code. 3. **Strict TypeScript** — `tsconfig.json` is strict (incl. `noUncheckedIndexedAccess`, - `exactOptionalPropertyTypes`, `verbatimModuleSyntax`). Keep it that way. Prefer - exact types and limit nullable and multi option types when possible. KISS. -4. **Environment-agnostic** — the app never asks *which environment* it runs in; there is - no `NODE_ENV` (or equivalent) branching. Every behaviour is an **explicit config - toggle** (e.g. `CACHE_TEMPLATES`, `REQUIRE_SECURE_SECRETS`, a future "disable email"), - read once in `src/config.ts`. Compose files set the toggles per deployment. -5. **Semantic, accessible DOM** — markup is a first-class concern. Use the right element - for the job (landmarks, one `

` per page + sane heading order, lists, `` with - row/column headers, `
`/``, `
` with row/column headers, `
`/``, + `