Run the E2E runner as the invoking user so its artifacts aren't root-owned
CI / full-gate (push) Successful in 2m37s

This commit is contained in:
2026-08-05 22:25:17 +02:00
parent e8b91ecd09
commit 073ec294e9
14 changed files with 61 additions and 21 deletions
+1 -1
View File
@@ -2,7 +2,6 @@
## Unfinnished work
- [ ] `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,6 +37,7 @@ Prioritized. Overall verdict: architecture is sound (contract-first plugin API,
## Finnished work
- [x] `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. (Fixed with the idiom README already uses for a lockfile edit — every documented invocation passes `--user "$(id -u):$(id -g)"`: the five compose headers, the five README blocks and `ci.sh`'s two runs — so the runner writes as whoever started it, on a dev box and on a CI runner alike, and compose never needs a `$UID` of its own. Two things had to come with it. `e2e-tests/artifacts/` is now *tracked* (`.gitkeep`), because an absent bind-mount source is created by the daemon as root and an unprivileged runner then cannot write into it at all — the same trap the node_modules mountpoint hit, one layer up. And the runner image sets `HOME=/tmp` + `npm_config_cache=/tmp/.npm`, since an arbitrary uid has no home in the Playwright image. Baking a `USER` into the image was tried first and dropped: `pwuser` is **1001** in the noble image (uid 1000 is `ubuntu`), so it `EACCES`'d on a 1000-owned checkout, and no fixed uid can match every host. One premise turned out stale — no workflow uploads artifacts, so nothing in CI reads that dir. Verified by running the visual suite as uid 1000: 36 tests green across Chromium, Firefox and WebKit, every file written `lilleman:lilleman` and deletable without `sudo`, which this box does not even have. `src/compose.test.ts` guards every documented command plus the tracked mount point; `src/ci-gate.test.ts` guards the gate's own two.)
- [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.)