diff --git a/.gitignore b/.gitignore index aa5c1fc..617f6bd 100644 --- a/.gitignore +++ b/.gitignore @@ -3,8 +3,10 @@ *.log node_modules -# Playwright E2E outputs (screenshots, html report, traces) -e2e-tests/artifacts/ +# Playwright E2E outputs (screenshots, html report, traces). The dir itself is tracked: an absent +# bind-mount source is created by the daemon as root, which the unprivileged runner cannot write. +/e2e-tests/artifacts/* +!/e2e-tests/artifacts/.gitkeep # plugins/ is a drop-in mount point, not committed code — keep it empty (see examples/plugins/ for the reference) /plugins/* diff --git a/AGENTS.md b/AGENTS.md index c2a6176..faa5844 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -224,6 +224,14 @@ Revisit only if the stated reason stops holding. 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). +- **A container that writes into the checkout runs as `--user "$(id -u):$(id -g)"`** — the E2E runner + (artifacts) and a lockfile edit both do, or the output is root-owned and needs `sudo` to delete, + which a dev box may not have at all. Two consequences: `e2e-tests/artifacts/` is *tracked* + (`.gitkeep`), since an absent bind-mount source is daemon-created as root and that uid then cannot + write it; and the runner image points `HOME` + npm's cache at `/tmp`, since an arbitrary uid has no + home in it. Baking a `USER` in instead does not work — the image's `pwuser` is 1001, and no fixed + uid matches every host. `src/compose.test.ts` guards every documented command, `src/ci-gate.test.ts` + the gate's own. - **Anything the browser logs fails the E2E test that provoked it.** Every spec takes its `test` from `e2e-tests/console-guard.ts`, which fails a test on a console error/warning or uncaught exception on any page it opened. A zero-JS app has nothing to say in the console, so the bar is *zero* rather than diff --git a/README.md b/README.md index 682e805..6251f2f 100644 --- a/README.md +++ b/README.md @@ -1490,7 +1490,7 @@ plugin permission-gating — the last exercised by bind-mounting the reference e the plugin's own pages, an `Accept-Language`-only visitor, and the fallback for an uninstalled locale. ```bash -docker compose -f compose.yml -f e2e-tests/compose.visual.yml run --build --rm e2e # run the suite +docker compose -f compose.yml -f e2e-tests/compose.visual.yml run --user "$(id -u):$(id -g)" --build --rm e2e # run the suite docker compose -f compose.yml -f e2e-tests/compose.visual.yml down -v # tear down after ``` @@ -1502,7 +1502,7 @@ live Kratos session (permissions re-read from Keto), and once that session is re cookie is **cleared**. ```bash -docker compose -f compose.yml -f e2e-tests/compose.auth.yml run --build --rm e2e # run the suite +docker compose -f compose.yml -f e2e-tests/compose.auth.yml run --user "$(id -u):$(id -g)" --build --rm e2e # run the suite docker compose -f compose.yml -f e2e-tests/compose.auth.yml down -v # tear down after ``` @@ -1514,7 +1514,7 @@ then shows the consent screen for the third-party client and **Allow** drives Hy the authorization code. ```bash -docker compose -f compose.yml -f e2e-tests/compose.oauth.yml run --build --rm e2e # run the suite +docker compose -f compose.yml -f e2e-tests/compose.oauth.yml run --user "$(id -u):$(id -g)" --build --rm e2e # run the suite docker compose -f compose.yml -f e2e-tests/compose.oauth.yml down -v # tear down after ``` @@ -1528,7 +1528,7 @@ Kratos on one host (`ory/kratos/e2e-proxy.yml` points Kratos at it) — exactly reverse proxy would. ```bash -docker compose -f compose.yml -f e2e-tests/compose.full.yml run --build --rm e2e # run the suite +docker compose -f compose.yml -f e2e-tests/compose.full.yml run --user "$(id -u):$(id -g)" --build --rm e2e # run the suite docker compose -f compose.yml -f e2e-tests/compose.full.yml down -v # tear down after ``` @@ -1546,11 +1546,12 @@ proxied full-flow suite can't catch this (it fronts web + Kratos on one origin). `ci.sh` — it needs host networking and the host ports `3000`/`4433` free (Linux). ```bash -docker compose -f compose.yml -f compose.override.yml -f e2e-tests/compose.devstack.yml run --build --rm e2e # run it +docker compose -f compose.yml -f compose.override.yml -f e2e-tests/compose.devstack.yml run --user "$(id -u):$(id -g)" --build --rm e2e # run it docker compose -f compose.yml -f compose.override.yml -f e2e-tests/compose.devstack.yml down -v # tear down ``` -Screenshots + an HTML report land in `e2e-tests/artifacts/` (git-ignored). Every user-facing flow +Screenshots + an HTML report land in `e2e-tests/artifacts/` (git-ignored; `--user` above is what +keeps them yours to delete — the runner writes them into your checkout). Every user-facing flow is covered end-to-end; tests are independent and run **fully in parallel** for speed ([AGENTS.md](AGENTS.md)) — keep new tests side-effect-free so the suite stays fast. diff --git a/ci.sh b/ci.sh index fa62461..e9782d1 100755 --- a/ci.sh +++ b/ci.sh @@ -62,11 +62,13 @@ count=$(echo "$units" | grep -oE 'tests [0-9]+' | grep -oE '[0-9]+' | head -1 || # Run one E2E suite against its OWN named stack, then always tear it down (even on failure). The # per-suite project name keeps a flaky teardown from leaking containers/volumes into the next suite. +# --user: the runner writes screenshots + the report into the checkout, so they must belong to +# whoever ran the gate — root-owned output needs sudo to delete, and a dev box may have none. e2e() { step "E2E: $1" local proj="plainpages-e2e-$(basename "$1" .yml | tr '.' '-')" # dots aren't valid in a compose project name local rc=0 - docker compose -p "$proj" -f compose.yml -f "$1" run --build --rm e2e || rc=$? + docker compose -p "$proj" -f compose.yml -f "$1" run --user "$(id -u):$(id -g)" --build --rm e2e || rc=$? docker compose -p "$proj" -f compose.yml -f "$1" down -v >/dev/null 2>&1 || true [ "$rc" -eq 0 ] || { echo "E2E suite $1 failed (exit $rc)"; exit "$rc"; } } @@ -82,7 +84,7 @@ e2e e2e-tests/compose.full.yml # full browser flow: login (password + SSO), step "E2E: e2e-tests/compose.devstack.yml (dev-stack login: localhost works + 127.0.0.1 canonicalised)" devstack_files=(-f compose.yml -f compose.override.yml -f e2e-tests/compose.devstack.yml) rc=0 -docker compose -p plainpages-e2e-devstack "${devstack_files[@]}" run --build --rm e2e || rc=$? +docker compose -p plainpages-e2e-devstack "${devstack_files[@]}" run --user "$(id -u):$(id -g)" --build --rm e2e || rc=$? docker compose -p plainpages-e2e-devstack "${devstack_files[@]}" down -v >/dev/null 2>&1 || true [ "$rc" -eq 0 ] || { echo "E2E suite e2e-tests/compose.devstack.yml failed (exit $rc)"; exit "$rc"; } diff --git a/e2e-tests/Dockerfile b/e2e-tests/Dockerfile index 40fae2a..bdb914b 100644 --- a/e2e-tests/Dockerfile +++ b/e2e-tests/Dockerfile @@ -9,4 +9,9 @@ RUN npm ci COPY e2e-tests/ ./ +# Runs as the invoking `--user` so artifacts land owned by them, not root — that uid has no home +# here, so point everything that wants one at /tmp (npm's cache, and the browsers' profile dirs). +ENV HOME=/tmp +ENV npm_config_cache=/tmp/.npm + CMD ["npx", "playwright", "test"] diff --git a/e2e-tests/artifacts/.gitkeep b/e2e-tests/artifacts/.gitkeep new file mode 100644 index 0000000..e69de29 diff --git a/e2e-tests/compose.auth.yml b/e2e-tests/compose.auth.yml index 764f64e..dbd902b 100644 --- a/e2e-tests/compose.auth.yml +++ b/e2e-tests/compose.auth.yml @@ -2,7 +2,7 @@ # visual suite (e2e-tests/compose.visual.yml) covers the design system; this is its full-stack counterpart: # real Postgres + Kratos + Keto + bootstrap + web, with a SHORT tokenizer TTL (ory/kratos/e2e.yml) # and zero clock skew, so the JWT lapses and re-mints within seconds instead of ~10m. -# docker compose -f compose.yml -f e2e-tests/compose.auth.yml run --build --rm e2e +# docker compose -f compose.yml -f e2e-tests/compose.auth.yml run --user "$(id -u):$(id -g)" --build --rm e2e # docker compose -f compose.yml -f e2e-tests/compose.auth.yml down -v # tear down after services: web: diff --git a/e2e-tests/compose.devstack.yml b/e2e-tests/compose.devstack.yml index 9f5b405..669dcbb 100644 --- a/e2e-tests/compose.devstack.yml +++ b/e2e-tests/compose.devstack.yml @@ -4,7 +4,7 @@ # runs against the *plain* `docker compose up` topology and drives the browser on the HOST network, so # it sees http://localhost:3000 (web) and http://127.0.0.1:4433 (Kratos public) exactly as a host # browser does. Merge the dev override so the live stack is byte-for-byte `docker compose up`: -# docker compose -f compose.yml -f compose.override.yml -f e2e-tests/compose.devstack.yml run --build --rm e2e +# docker compose -f compose.yml -f compose.override.yml -f e2e-tests/compose.devstack.yml run --user "$(id -u):$(id -g)" --build --rm e2e # docker compose -f compose.yml -f compose.override.yml -f e2e-tests/compose.devstack.yml down -v # tear down services: web: diff --git a/e2e-tests/compose.full.yml b/e2e-tests/compose.full.yml index 0c07d59..7285000 100644 --- a/e2e-tests/compose.full.yml +++ b/e2e-tests/compose.full.yml @@ -2,7 +2,7 @@ # login, menu filtering by permission, users/groups/OAuth2-clients CRUD + permission granting, a plugin page, logout. A # tiny same-origin gateway (proxy, e2e-tests/proxy.ts) fronts web + Kratos on one host so the browser's cookies # round-trip (ory/kratos/e2e-proxy.yml points Kratos at it); a mock OIDC provider backs the SSO test. -# docker compose -f compose.yml -f e2e-tests/compose.full.yml run --build --rm e2e +# docker compose -f compose.yml -f e2e-tests/compose.full.yml run --user "$(id -u):$(id -g)" --build --rm e2e # docker compose -f compose.yml -f e2e-tests/compose.full.yml down -v # tear down after services: web: diff --git a/e2e-tests/compose.oauth.yml b/e2e-tests/compose.oauth.yml index 97269af..6232375 100644 --- a/e2e-tests/compose.oauth.yml +++ b/e2e-tests/compose.oauth.yml @@ -3,7 +3,7 @@ # it via the Kratos session and accepts. Runs against the real stack (Postgres + Kratos + Keto + # Hydra + bootstrap + web). The runner drives the flow over HTTP (fetch, manual cookies), so it # reaches the Ory services by their compose-network names. -# docker compose -f compose.yml -f e2e-tests/compose.oauth.yml run --build --rm e2e +# docker compose -f compose.yml -f e2e-tests/compose.oauth.yml run --user "$(id -u):$(id -g)" --build --rm e2e # docker compose -f compose.yml -f e2e-tests/compose.oauth.yml down -v # tear down after services: web: diff --git a/e2e-tests/compose.visual.yml b/e2e-tests/compose.visual.yml index c2bca97..f309e12 100644 --- a/e2e-tests/compose.visual.yml +++ b/e2e-tests/compose.visual.yml @@ -1,7 +1,7 @@ # Playwright E2E. Brings up the app + a Playwright runner and exercises the live pages (design # system, theme switch, mobile layout, CSRF, landing, 404, plugin gating, language switching) — # Ory-free, so it's fast. -# docker compose -f compose.yml -f e2e-tests/compose.visual.yml run --build --rm e2e +# docker compose -f compose.yml -f e2e-tests/compose.visual.yml run --user "$(id -u):$(id -g)" --build --rm e2e # docker compose -f compose.yml -f e2e-tests/compose.visual.yml down -v # tear down after # --build rebuilds the runner (the image bakes in e2e-tests/) so spec edits are picked up. # Screenshots + HTML report land in ./e2e-tests/artifacts/ (git-ignored). diff --git a/src/ci-gate.test.ts b/src/ci-gate.test.ts index e52de3d..b4956d2 100644 --- a/src/ci-gate.test.ts +++ b/src/ci-gate.test.ts @@ -29,6 +29,14 @@ test("the commit-hash image is pushed even when the gate no-ops", () => { assert.doesNotMatch(step("docker push"), /^\s*if:/m); }); +test("every E2E suite the gate runs writes its artifacts as the invoking user", () => { + // The documented hand-run commands carry the same flag (src/compose.test.ts). + const runs = gate.split("\n").filter((l) => /docker compose .*\brun\b.*\be2e\b/.test(l)); + assert.equal(runs.length, 2, "the suite helper and the devstack run"); + for (const line of runs) + assert.match(line, /--user "\$\(id -u\):\$\(id -g\)"/, `runs as the caller: ${line.trim()}`); +}); + test("only *.md counts as docs; a dirty tree and a rename both count as changed", () => { assert.ok(gate.includes("\\.md$"), "the non-docs match is a *.md suffix test"); assert.match(gate, /git status --porcelain --no-renames/, "uncommitted code and a staged rename can never be skipped over"); diff --git a/src/compose.test.ts b/src/compose.test.ts index 340f014..fea487d 100644 --- a/src/compose.test.ts +++ b/src/compose.test.ts @@ -6,9 +6,13 @@ // by running the stack; this catches edits. import { test } from "node:test"; import assert from "node:assert/strict"; -import { readFileSync, readdirSync } from "node:fs"; +import { existsSync, readFileSync, readdirSync } from "node:fs"; const read = (p: string) => readFileSync(new URL(`../${p}`, import.meta.url), "utf8"); +const composeFiles = (dir: string) => + readdirSync(new URL(`../${dir}`, import.meta.url)) + .filter((f) => f.startsWith("compose.") && f.endsWith(".yml")) + .map((f) => `${dir}${f}`); const compose = read("compose.yml"); const override = read("compose.override.yml"); const visual = read("e2e-tests/compose.visual.yml"); @@ -98,14 +102,24 @@ test("deps live above WORKDIR, so no mount creates a root-owned dir in the check 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 = (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`); }); +test("the E2E runner writes its artifacts as the invoking user, never as root", () => { + // Same trap as the node_modules mountpoint above, but the runner must write into the checkout, + // so the fix is the uid: root-owned output needs sudo to delete, which a dev box may not have. + const documented = [read("README.md"), ...composeFiles("e2e-tests/").map(read)] + .join("\n").split("\n").filter((l) => /docker compose .*--rm e2e\b/.test(l)); + assert.ok(documented.length >= 5, "every suite's run command is documented"); + for (const l of documented) + assert.match(l, /--user "\$\(id -u\):\$\(id -g\)"/, `passes the uid: ${l.trim()}`); + // An absent mount source is daemon-created as root, and then that uid can't write it at all. + assert.ok(existsSync(new URL("../e2e-tests/artifacts/.gitkeep", import.meta.url)), + "the mount point exists in the checkout"); + assert.match(read(".gitignore"), /^!\/e2e-tests\/artifacts\/\.gitkeep$/m, "and stays tracked"); +}); + 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/todo.md b/todo.md index 737be68..4fb7809 100644 --- a/todo.md +++ b/todo.md @@ -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: :, 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.)