Cut the node_modules prose to one home each; drop a stray tracked file
CI / full-gate (push) Successful in 2m38s
CI / full-gate (push) Successful in 2m38s
This commit is contained in:
@@ -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 `<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] 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: <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.)
|
||||
|
||||
Reference in New Issue
Block a user