Harden the artifact-ownership guards and document the root-owned upgrade trap
CI / full-gate (push) Successful in 2m38s

This commit is contained in:
2026-08-05 22:40:36 +02:00
parent 073ec294e9
commit 1cf34a0d45
4 changed files with 31 additions and 15 deletions
+11 -8
View File
@@ -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 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 sit at that path now — it shadows `/node_modules` silently (`src/compose.test.ts` guards the compose
files, `.dockerignore` the image). files, `.dockerignore` the image).
- **A container that writes into the checkout runs as `--user "$(id -u):$(id -g)"`** — the E2E runner - **A container whose output a human then edits or deletes runs as `--user "$(id -u):$(id -g)"`** —
(artifacts) and a lockfile edit both do, or the output is root-owned and needs `sudo` to delete, the E2E runner (artifacts) and a lockfile edit, or the output is root-owned and needs `sudo`, which
which a dev box may not have at all. Two consequences: `e2e-tests/artifacts/` is *tracked* a dev box may not have at all. Not universal: `bootstrap` writes `jwks.json` as root on a first
(`.gitkeep`), since an absent bind-mount source is daemon-created as root and that uid then cannot boot, and a generated signing key is the operator's to leave alone. Three consequences.
write it; and the runner image points `HOME` + npm's cache at `/tmp`, since an arbitrary uid has no `e2e-tests/artifacts/` is *tracked* (`.gitkeep`), since an absent bind-mount source is
home in it. Baking a `USER` in instead does not work — the image's `pwuser` is 1001, and no fixed daemon-created as root and that uid then cannot write it — which also makes a root-owned leftover
uid matches every host. `src/compose.test.ts` guards every documented command, `src/ci-gate.test.ts` an upgrade hazard (README → Breaking changes). The runner image sets `HOME=/tmp`, since an
the gate's own. 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 - **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 `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 any page it opened. A zero-JS app has nothing to say in the console, so the bar is *zero* rather than
+11 -1
View File
@@ -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 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 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. ([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 - **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 pulling (it is empty — no `sudo`) and `docker volume prune`. Keep that path clear: anything there
silently shadows the image's deps. 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 ## Observability
+2 -3
View File
@@ -9,9 +9,8 @@ RUN npm ci
COPY e2e-tests/ ./ COPY e2e-tests/ ./
# Runs as the invoking `--user` so artifacts land owned by them, not root — that uid has no home # Runs as the invoking `--user` so artifacts land owned by them, not root — and an arbitrary uid has
# here, so point everything that wants one at /tmp (npm's cache, and the browsers' profile dirs). # no passwd entry here, so its home would be the unwritable `/`. npm's cache follows HOME.
ENV HOME=/tmp ENV HOME=/tmp
ENV npm_config_cache=/tmp/.npm
CMD ["npx", "playwright", "test"] CMD ["npx", "playwright", "test"]
+7 -3
View File
@@ -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", () => { 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, // 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. // 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)] const documented = [read("README.md"), ...composeFiles("e2e-tests/").map(read)]
.join("\n").split("\n").filter((l) => /docker compose .*--rm e2e\b/.test(l)); .join("\n").split("\n").filter((l) => /docker compose .*\brun\b.*\be2e\b/.test(l));
assert.ok(documented.length >= 5, "every suite's run command is documented"); assert.equal(documented.length, 10, "5 compose headers + 5 README blocks");
for (const l of documented) for (const l of documented)
assert.match(l, /--user "\$\(id -u\):\$\(id -g\)"/, `passes the uid: ${l.trim()}`); 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. // 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)), assert.ok(existsSync(new URL("../e2e-tests/artifacts/.gitkeep", import.meta.url)),
"the mount point exists in the checkout"); "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", () => { test("the visual E2E does not drag in the Ory stack", () => {