From 1cf34a0d450bfe831c8744b918eb923aa43aa953 Mon Sep 17 00:00:00 2001 From: lilleman Date: Wed, 5 Aug 2026 22:40:36 +0200 Subject: [PATCH] Harden the artifact-ownership guards and document the root-owned upgrade trap --- AGENTS.md | 19 +++++++++++-------- README.md | 12 +++++++++++- e2e-tests/Dockerfile | 5 ++--- src/compose.test.ts | 10 +++++++--- 4 files changed, 31 insertions(+), 15 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index faa5844..3362077 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -224,14 +224,17 @@ 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. +- **A container whose output a human then edits or deletes runs as `--user "$(id -u):$(id -g)"`** — + the E2E runner (artifacts) and a lockfile edit, or the output is root-owned and needs `sudo`, which + a dev box may not have at all. Not universal: `bootstrap` writes `jwks.json` as root on a first + boot, and a generated signing key is the operator's to leave alone. Three 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 — which also makes a root-owned leftover + an upgrade hazard (README → Breaking changes). The runner image sets `HOME=/tmp`, since an + arbitrary uid has no passwd entry and would land on an unwritable `/`. And rootless Docker wants + the flag *dropped* — container root is already the invoking user there. 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 6251f2f..83648fc 100644 --- a/README.md +++ b/README.md @@ -1551,7 +1551,8 @@ docker compose -f compose.yml -f compose.override.yml -f e2e-tests/compose.devst ``` 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 +keeps them yours to delete — the runner writes them into your checkout). On **rootless** Docker drop +that flag: container root is already you there, and a mapped uid cannot write. 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. @@ -1772,6 +1773,15 @@ so for now the error names the rule it tripped instead. - **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. +- **`e2e-tests/artifacts/` is tracked, and the E2E runner writes as you** (2026-08-05). A root-owned + leftover from an earlier run blocks the checkout of its `.gitkeep` — git reports the failure but + still exits 0, leaving the file staged as deleted — and every E2E suite then fails `EACCES`. Clear + it before pulling; a root container does what `sudo` would, which this needs and a dev box may lack: + + ```bash + docker run --rm -v "$PWD/e2e-tests:/x" alpine:3.23 rm -rf /x/artifacts + git checkout -- e2e-tests/artifacts/.gitkeep + ``` ## Observability diff --git a/e2e-tests/Dockerfile b/e2e-tests/Dockerfile index bdb914b..770f2c7 100644 --- a/e2e-tests/Dockerfile +++ b/e2e-tests/Dockerfile @@ -9,9 +9,8 @@ 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). +# Runs as the invoking `--user` so artifacts land owned by them, not root — and an arbitrary uid has +# no passwd entry here, so its home would be the unwritable `/`. npm's cache follows HOME. ENV HOME=/tmp -ENV npm_config_cache=/tmp/.npm CMD ["npx", "playwright", "test"] diff --git a/src/compose.test.ts b/src/compose.test.ts index fea487d..1a96ba7 100644 --- a/src/compose.test.ts +++ b/src/compose.test.ts @@ -109,15 +109,19 @@ test("deps live above WORKDIR, so no mount creates a root-owned dir in the check 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. + // Matched independently of flag order, and counted: a reordered flag that slips out of the + // filter would otherwise leave that command silently unguarded. 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"); + .join("\n").split("\n").filter((l) => /docker compose .*\brun\b.*\be2e\b/.test(l)); + assert.equal(documented.length, 10, "5 compose headers + 5 README blocks"); 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"); + const gitignore = read(".gitignore"); + assert.match(gitignore, /^\/e2e-tests\/artifacts\/\*$/m, "its output stays ignored"); + assert.match(gitignore, /^!\/e2e-tests\/artifacts\/\.gitkeep$/m, "the mount point stays tracked"); }); test("the visual E2E does not drag in the Ory stack", () => {