Install deps above WORKDIR so no root-owned node_modules lands in the checkout
CI / full-gate (push) Successful in 2m45s

This commit is contained in:
2026-08-05 18:18:49 +02:00
parent 2852722873
commit bcf4d7fb1f
7 changed files with 53 additions and 13 deletions
+11
View File
@@ -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 `<repo>/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
+10 -5
View File
@@ -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"]
+11 -3
View File
@@ -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/<plugin>/<path>`.
- **New dependency:** `docker compose run --rm web npm install <pkg>` (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 <pkg>
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
-2
View File
@@ -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
+17 -1
View File
@@ -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");
+2 -1
View File
@@ -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 =>
+2 -1
View File
@@ -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 `<repo>/node_modules/...` and now resolves it by specifier via `import.meta.resolve`; the documented `npm install <pkg>` 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: <resource>:<action>, 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.)