test(e2e): wire three never-run specs into CI, and make the security one real - #377
Merged
Conversation
…one real
The e2e-local job runs 'tests/e2e/*journey.spec.ts'. Nine specs are not
named *journey, so nothing ran them — anywhere, ever. I ran all of them
against a locally seeded instance before wiring any, which is the only
reason this PR is three specs and not nine.
Wired (41 assertions that were running nowhere):
security 4 tests — rewritten, see below
notification-hrefs 21 assertions, green as-is
user-admin-flows 16 assertions, green after one fix
security.spec.ts was a STUB and wiring it as-written would have been
worse than leaving it out: two of its four tests had comment-only bodies
('Would need to fill and submit form 6 times'), a third asserted only
inside an isVisible() branch, and every one called test.skip() when
redirected to login — which /it-hilfe/create always does signed out. In
CI it would have reported a green 'security' check while exercising
nothing. It now asserts the closed side of the authorization boundary: no
admin page renders and no admin/money API answers 2xx to a signed-out
request, and a refused response carries no user data. Proven by mutation
— adding a public route to the protected list turns it red.
user-admin-flows addressed routes as /de/x. The app 307-redirects those
to /x, and when that races the client router Playwright aborts the
navigation (net::ERR_ABORTED) — indistinguishable from a broken page.
It cost three wrong hypotheses (concurrency, then state residue, then a
fresh database) before I read the error. Now uses the canonical paths,
which is where the redirect lands and what users actually see.
NOT wired, with reasons rather than silence: marketplace, it-hilfe,
appointments, payment-return and timecards fail against the current app —
marketplace expects an h1 of 'Marketplace' and the text 'gebrauchte
IT-Geräte', which is pre-rebrand Revamp-IT copy. dashboard-timecards
skips its only test. Specs nothing runs rot; these did. Recorded in
docs/AUDIT_BACKLOG_2026-08.md.
Verified: 3 consecutive green runs of the bundle, typecheck, lint, docs gate.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Three are now wired; the other six are not, and the table says why rather than leaving a silent gap. marketplace expects pre-rebrand copy, and dashboard-timecards skips its only test. it-hilfe is the best next candidate — 10 of its 14 tests already pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
CI rejected the previous commit and it was right to. playwright.config.ts sets storageState globally, so every context — the page AND the request fixture — inherits a saved login unless a spec says otherwise. My 'authorization boundary (signed out)' tests therefore ran authenticated: /api/invoices, /api/admin/users and /api/admin/refunds all answered 200, and the spec reported the doors were open. They passed locally only because no saved-session file exists there. That is the worst kind of green: right answer, wrong reason, and dependent on which machine you run it on. Both specs now force an empty session explicitly. For security.spec.ts that line IS the test — a signed-out check carrying a session asserts nothing about the closed side of the boundary. Same root cause fixed notification-hrefs: it probes whether a ROUTE exists using a dummy UUID matching no record. Signed out, an auth redirect answers that without touching the database. Signed in, the admin page renders and correctly 404s on the missing record — so it reported 'notification deep link is broken' when nothing was broken. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to the audit: the
e2e-localjob runstests/e2e/*journey.spec.ts. Nine specs aren't named*journey, so nothing ran them — anywhere, ever.I ran all nine against a locally seeded instance before wiring any. That's the only reason this PR is three specs and not nine.
Wired — 41 assertions that were running nowhere
securitynotification-hrefsuser-admin-flowssecurity.spec.tswas a stub — wiring it as-written would have been worse than leaving it outTwo of its four tests had comment-only bodies ("Would need to fill and submit form 6 times"), a third asserted only inside an
isVisible()branch, and every one calledtest.skip()when redirected to login — which/it-hilfe/createalways does signed out. In CI it would have reported a green "security" check while exercising nothing.It now asserts the closed side of the authorization boundary — the same boundary where #358 found a real privilege escalation:
It needs no credentials, so it can't be skipped for want of secrets. Proven by mutation: adding a public route to the protected list turns it red.
The one fix in
user-admin-flowsIt addressed routes as
/de/x. The app 307-redirects those to/x, and when that redirect races the client router Playwright aborts the navigation (net::ERR_ABORTED) — indistinguishable from a broken page. It cost me three wrong hypotheses (concurrency → state residue → fresh database) before I stopped guessing and read the error context. Now uses the canonical paths, which is where the redirect lands and what users actually see.Not wired, with reasons rather than silence
marketplaceh1= "Marketplace" and "gebrauchte IT-Geräte" — pre-rebrand Revamp-IT copyit-hilfeappointmentspayment-returntimecardsdashboard-timecardsSpecs that nothing runs rot. These did. Recorded in
docs/AUDIT_BACKLOG_2026-08.mdwith the evidence.Verification
3 consecutive green runs of the bundle against a freshly migrated + seeded database (41/41 each), typecheck clean, lint 0 errors, docs gate green.
🤖 Generated with Claude Code