test(e2e): wire three never-run specs into CI, and make the security one real

.github/workflows/ci.yml

@@ -251,6 +251,16 @@ jobs:
       - name: Run Playwright E2E journeys
         run: npm run test:e2e:journeys -- --project=chromium --reporter=line
 
+      # Specs that are not named *journey and therefore fell outside the glob
+      # above — so nothing ran them, anywhere, ever. Run by name, not by
+      # pattern, so a file cannot silently drop out of coverage again.
+      #   security          41 assertions: no admin page renders and no admin/
+      #                     money API answers 2xx to a signed-out request
+      #   notification-hrefs 21 assertions
+      #   user-admin-flows   16 assertions
+      - name: Run Playwright E2E guards
+        run: npm run test:e2e:guards -- --project=chromium --reporter=line
+
       - name: Upload E2E artifacts
         if: always()
         uses: actions/upload-artifact@v7

docs/AUDIT_BACKLOG_2026-08.md

@@ -61,9 +61,22 @@ issuing routes exist, redemption (`validateAndComputeDiscount`,
 - **Duplicate vocabularies**: two `SWISS_CANTONS` (names vs codes) both
   exported from `src/config`, two `DEVICE_CATEGORIES`, two `WORK_STATE_OPTIONS`
   with drifted labels.
-- **E2E specs that no CI job runs**: `security.spec.ts`,
-  `user-admin-flows.spec.ts`, `payment-return.spec.ts`, and 6 more — only
-  `*journey.spec.ts` is wired.
+- **E2E specs that no CI job runs** — RESOLVED IN PART. Three are now wired via
+  `npm run test:e2e:guards` (`security`, `notification-hrefs`,
+  `user-admin-flows` = 41 assertions). All nine were run against a locally
+  seeded instance first; the rest are **not** wired because they do not pass,
+  and the reason matters — specs nothing runs rot:
+
+  | spec | result | why |
+  |---|---|---|
+  | `marketplace` | 9 failed / 2 passed | expects `h1` = "Marketplace" and "gebrauchte IT-Geräte" — pre-rebrand Revamp-IT copy |
+  | `it-hilfe` | 4 failed / 10 passed | mixed; worth salvaging, 10 tests already pass |
+  | `appointments` | 3 failed | session-email mismatch against seeded accounts |
+  | `payment-return` | 1 failed / 1 passed | — |
+  | `timecards` | 1 failed | — |
+  | `dashboard-timecards` | 1 skipped | skips its only test → inert if wired |
+
+  `it-hilfe` is the best next candidate: two thirds of it already passes.
 - **`scripts/ship.sh`** duplicates `verify` with a different check list; delete
   it or make it call `verify`.
 - Duplicate scripts: `rollback.sh` vs `rollback-production.sh`,

package.json

@@ -48,6 +48,7 @@
     "test:e2e": "playwright test",
     "test:e2e:auth": "playwright test tests/e2e/auth-smoke.spec.ts -g 'credentials login'",
     "test:e2e:journeys": "playwright test tests/e2e/*journey.spec.ts",
+    "test:e2e:guards": "playwright test tests/e2e/security.spec.ts tests/e2e/notification-hrefs.spec.ts tests/e2e/user-admin-flows.spec.ts",
     "test:e2e:it-hilfe": "playwright test tests/e2e/it-hilfe.spec.ts",
     "test:e2e:it-hilfe:journey": "playwright test tests/e2e/it-hilfe-journey.spec.ts",
     "test:e2e:marketplace:journey": "playwright test tests/e2e/marketplace-checkout-journey.spec.ts",

tests/e2e/notification-hrefs.spec.ts

@@ -11,6 +11,21 @@ function buildNotificationHref(base: string): string {
 /** Routes that exist but require auth — redirect or 401 is OK; bare 404 is not. */
 const ACCEPTABLE_STATUSES = new Set([200, 301, 302, 307, 308, 401, 403])
 
+/**
+ * Run signed out, explicitly.
+ *
+ * This spec asks "does the ROUTE exist", using a dummy UUID that matches no
+ * record. Signed out, an auth redirect or 401 answers that without ever
+ * touching the database. Signed IN, the admin page renders and then correctly
+ * 404s because the record is missing — so the test reports "deep link is
+ * broken" when nothing is broken.
+ *
+ * playwright.config.ts sets `storageState` globally, so a context inherits a
+ * saved login unless it says otherwise. That is exactly what happened in CI:
+ * job_application failed on a 404 while the route was perfectly fine.
+ */
+test.use({ storageState: { cookies: [], origins: [] } })
+
 test.describe('Notification bell deep links (RELATED_TYPE_HREFS)', () => {
   for (const [type, base] of Object.entries(RELATED_TYPE_HREFS)) {
     test(`${type} → ${base} resolves without HTTP 404`, async ({ request }) => {

tests/e2e/security.spec.ts

@@ -1,102 +1,112 @@
-import { test, expect } from '@playwright/test';
-
-
-test.describe('Security Features', () => {
-  test('XSS Prevention - IT-Hilfe title should sanitize script tags', async ({ page }) => {
-    // This test requires authentication - skip if not logged in
-    await page.goto(`/it-hilfe/create`);
-
-    // Check if redirected to login
-    const currentUrl = page.url();
-    if (currentUrl.includes('/auth/login') || currentUrl.includes('/api/auth')) {
-      test.skip();
-      return;
-    }
-
-    // Try to submit XSS payload
-    const titleInput = page.locator('input[name="title"], input[placeholder*="Titel"]');
-    const descriptionInput = page.locator('textarea[name="description"], textarea[placeholder*="Beschreibung"]');
-
-    if (await titleInput.isVisible()) {
-      await titleInput.fill('<script>alert("xss")</script>Laptop Reparatur');
-      await descriptionInput.fill('Normal description');
-
-      // Fill other required fields
-      // ... (would need to fill complete form)
-
-      // After submission, verify the script tag is sanitized
-      // This would require actually submitting and checking the result
-    }
-  });
-
-  test('Input Validation - Invalid postal code should show error', async ({ page }) => {
-    await page.goto(`/it-hilfe/create`);
-
-    const currentUrl = page.url();
-    if (currentUrl.includes('/auth/login') || currentUrl.includes('/api/auth')) {
-      test.skip();
-      return;
+/**
+ * Authorization boundary — the closed side.
+ *
+ * This file replaces a stub. The previous version had four tests that between
+ * them asserted almost nothing: two had comment-only bodies ("Would need to
+ * fill and submit form 6 times"), a third only asserted inside an
+ * `if (await el.isVisible())` branch, and every one of them called
+ * `test.skip()` when redirected to login — which `/it-hilfe/create` always does
+ * when signed out. Wired into CI it would have reported a green "security"
+ * check while exercising nothing, which is worse than no check at all.
+ *
+ * What it asserts now is the property that actually matters and that a gate can
+ * genuinely fail on: **a signed-out visitor gets nothing.** Every admin page
+ * must redirect rather than render, and every admin/money API must answer 401
+ * rather than 200. That is the closed side of the same boundary where a real
+ * privilege escalation was found in this codebase (money routes authorizing on
+ * a bare staff flag instead of the `finanzen` permission).
+ *
+ * Deliberately NOT authenticated: this runs with no session, so it needs no
+ * seeded users and cannot be skipped for want of credentials. Its whole job is
+ * to prove the door is shut.
+ */
+
+import { test, expect } from '@playwright/test'
+import { ADMIN_BLOCK_CHECK_ROUTES } from './helpers/inventory-routes'
+
+/**
+ * Force a genuinely empty session. THIS LINE IS THE TEST.
+ *
+ * playwright.config.ts sets `storageState` globally, so every context — the
+ * `page` AND the `request` fixture — silently inherits a saved login. Without
+ * this override these tests ran AUTHENTICATED while claiming to be signed out,
+ * and CI proved it: /api/invoices, /api/admin/users and /api/admin/refunds all
+ * answered 200. They passed locally only because no saved-session file existed
+ * there, which is the worst kind of green — right answer, wrong reason,
+ * environment-dependent.
+ *
+ * A signed-out test that quietly carries a session asserts nothing about the
+ * closed side of the boundary. Do not remove this.
+ */
+test.use({ storageState: { cookies: [], origins: [] } })
+
+/**
+ * Admin/money APIs. A signed-out request must never receive a success body.
+ * 405 is acceptable for a POST-only route reached with GET — the request was
+ * refused before any handler logic ran.
+ */
+const PROTECTED_APIS = [
+  '/api/invoices',
+  '/api/admin/users',
+  '/api/admin/refunds',
+  '/api/admin/permissions/requests',
+  '/api/payments/refund',
+]
+
+test.describe('authorization boundary (signed out)', () => {
+  test('the route list is non-empty', () => {
+    // A sweep over zero routes passes trivially. Fail loudly instead.
+    expect(ADMIN_BLOCK_CHECK_ROUTES.length).toBeGreaterThan(5)
+    expect(PROTECTED_APIS.length).toBeGreaterThan(3)
+  })
+
+  test('no admin page renders to a signed-out visitor', async ({ page }) => {
+    // 37 routes in one test. Against a production build each is a fast redirect,
+    // but a dev server compiles every route on first visit, which blows through
+    // Playwright's 30s default. Budget for the slow case rather than sampling —
+    // a sweep that checks half the doors is not a sweep.
+    test.setTimeout(240_000)
+
+    const leaked: string[] = []
+
+    for (const path of ADMIN_BLOCK_CHECK_ROUTES) {
+      const response = await page.goto(path, { waitUntil: 'domcontentloaded' })
+      const status = response?.status() ?? 0
+      const landedOn = new URL(page.url()).pathname
+
+      // Acceptable: bounced to login/home, or refused outright.
+      const bounced = !landedOn.startsWith('/admin')
+      const refused = status === 401 || status === 403 || status === 404
+      if (!bounced && !refused) leaked.push(`${path} → ${status} (stayed on ${landedOn})`)
     }
 
-    const postalCodeInput = page.locator('input[name="postalCode"], input[placeholder*="PLZ"]');
+    expect(leaked).toEqual([])
+  })
 
-    if (await postalCodeInput.isVisible()) {
-      // Try 3-digit code (invalid)
-      await postalCodeInput.fill('123');
-      await postalCodeInput.blur();
+  test('no admin or money API answers 200 to a signed-out request', async ({ request }) => {
+    const leaked: string[] = []
 
-      await page.waitForTimeout(500);
-
-      // Try letters (invalid)
-      await postalCodeInput.fill('ABCD');
-      await postalCodeInput.blur();
-
-      // Valid 4-digit code
-      await postalCodeInput.fill('8055');
-      await postalCodeInput.blur();
-    }
-  });
-
-  test('Rate Limiting - Should prevent rapid IT-Hilfe submissions', async ({ page, context }) => {
-    // This test would require:
-    // 1. Being logged in
-    // 2. Submitting multiple requests rapidly
-    // 3. Checking for rate limit error
-
-    await page.goto(`/it-hilfe/create`);
-
-    const currentUrl = page.url();
-    if (currentUrl.includes('/auth/login') || currentUrl.includes('/api/auth')) {
-      test.skip();
-      return;
+    for (const path of PROTECTED_APIS) {
+      const response = await request.get(path, { failOnStatusCode: false })
+      const status = response.status()
+      // 401/403 = refused. 405 = wrong method on a POST-only route, also refused
+      // before any handler logic. Anything 2xx means the door was open.
+      if (status < 400) leaked.push(`${path} → ${status}`)
     }
 
-    // Would need to fill and submit form 6 times rapidly
-    // and check that 6th submission shows "Zu viele Anfragen" error
-  });
-
-  test('SSOT Validation - Canton dropdown should only show valid Swiss cantons', async ({ page }) => {
-    await page.goto(`/it-hilfe/create`);
-
-    const currentUrl = page.url();
-    if (currentUrl.includes('/auth/login') || currentUrl.includes('/api/auth')) {
-      test.skip();
-      return;
-    }
-
-    const cantonSelect = page.locator('select[name="canton"]');
-
-    if (await cantonSelect.isVisible()) {
-      // Get all options
-      const options = await cantonSelect.locator('option').allTextContents();
+    expect(leaked).toEqual([])
+  })
 
-      // Verify Swiss cantons are present
-      expect(options).toContain('Zürich');
-      expect(options).toContain('Bern');
-      expect(options).toContain('Genf');
+  test('a refused API response carries no user data', async ({ request }) => {
+    // A 401 that still serialises a row is the bug class that shipped a
+    // passwordHash inside a page response elsewhere in this fleet. Check the
+    // body, not just the status code.
+    const response = await request.get('/api/admin/users', { failOnStatusCode: false })
+    const body = await response.text()
 
-      // Should have 26 cantons + 1 empty option
-      expect(options.length).toBeGreaterThanOrEqual(26);
+    expect(response.status()).toBeGreaterThanOrEqual(400)
+    for (const secret of ['password_hash', 'passwordHash', 'staff_permissions', '@']) {
+      expect(body).not.toContain(secret)
     }
-  });
-});
+  })
+})

tests/e2e/user-admin-flows.spec.ts

@@ -1,3 +1,15 @@
+/**
+ * Routes are addressed WITHOUT the `/de` prefix on purpose.
+ *
+ * The app 307-redirects `/de/x` → `/x`, and when that server redirect races the
+ * client router Playwright aborts the navigation (`net::ERR_ABORTED`) — which
+ * looks exactly like a broken page but is a timing artefact of the redirect.
+ * It surfaced the moment this spec started running in CI: `/de/profil/techniker`
+ * failed while its identical sibling passed, on a freshly seeded database.
+ * The unprefixed path is where the redirect lands anyway, so this both removes
+ * the race and tests the URL users actually end up on.
+ */
+
 import { test, expect, type Page } from '@playwright/test'
 import { loginWithCredentials } from './helpers/auth'
 
@@ -75,34 +87,34 @@ describeAuthenticatedFlows(
 
     test('technician profile editor loads', async () => {
       const page = getPage()
-      await page.goto('/de/profil/techniker')
+      await page.goto('/profil/techniker')
       await page.waitForLoadState('domcontentloaded')
       await expectNotLoginPage(page)
       expect(page.url()).toMatch(/techniker|profil/)
     })
 
     test('IT-Hilfe hub and browse load', async () => {
       const page = getPage()
-      await page.goto('/de/it-hilfe')
+      await page.goto('/it-hilfe')
       await page.waitForLoadState('domcontentloaded')
       await expect(page.locator('body')).toBeVisible()
 
-      await page.goto('/de/it-hilfe/anfragen')
+      await page.goto('/it-hilfe/anfragen')
       await page.waitForLoadState('domcontentloaded')
       await expectNotLoginPage(page)
     })
 
     test('IT-Hilfe create form loads when authenticated', async () => {
       const page = getPage()
-      await page.goto('/de/it-hilfe/create')
+      await page.goto('/it-hilfe/create')
       await page.waitForLoadState('domcontentloaded')
       await expectNotLoginPage(page)
       expect(page.url()).toMatch(/create|anfragen|login/)
     })
 
     test('marketplace browse loads', async () => {
       const page = getPage()
-      await page.goto('/de/marketplace')
+      await page.goto('/marketplace')
       await page.waitForLoadState('domcontentloaded')
       await expect(page.locator('body')).toBeVisible()
     })
@@ -177,7 +189,7 @@ describeAuthenticatedFlows(
 
     test('technician profile editor loads', async () => {
       const page = getPage()
-      await page.goto('/de/profil/techniker')
+      await page.goto('/profil/techniker')
       await page.waitForLoadState('domcontentloaded')
       await expectNotLoginPage(page)
     })