Merge pull request #29 from builderz-labs/test/e2e-critical-fixes

Add 52 Playwright E2E tests for all critical fixes

.env.test

@@ -0,0 +1,7 @@
+AUTH_USER=testadmin
+AUTH_PASS=testpass123
+API_KEY=test-api-key-e2e-12345
+AUTH_SECRET=test-legacy-secret
+MC_ALLOW_ANY_HOST=1
+MC_COOKIE_SECURE=
+MC_COOKIE_SAMESITE=lax

.gitignore

@@ -29,6 +29,10 @@ next-env.d.ts
 .data/
 aegis/
 
+# Playwright
+test-results/
+playwright-report/
+
 # Claude Code context files
 CLAUDE.md
 **/CLAUDE.md

playwright.config.ts

@@ -9,10 +9,16 @@ export default defineConfig({
   fullyParallel: true,
   reporter: [['list']],
   use: {
-    baseURL: process.env.E2E_BASE_URL || 'http://127.0.0.1:3000',
+    baseURL: process.env.E2E_BASE_URL || 'http://127.0.0.1:3005',
     trace: 'retain-on-failure'
   },
   projects: [
     { name: 'chromium', use: { ...devices['Desktop Chrome'] } }
-  ]
+  ],
+  webServer: {
+    command: 'pnpm start',
+    url: 'http://127.0.0.1:3005',
+    reuseExistingServer: true,
+    timeout: 30_000,
+  }
 })

src/lib/migrations.ts

@@ -338,6 +338,12 @@ const migrations: Migration[] = [
   {
     id: '013_tenant_owner_gateway',
     up: (db) => {
+      // Check if tenants table exists (may not on fresh installs without super-admin)
+      const hasTenants = (db.prepare(
+        `SELECT name FROM sqlite_master WHERE type='table' AND name='tenants'`
+      ).get() as any)
+      if (!hasTenants) return
+
       const columns = db.prepare(`PRAGMA table_info(tenants)`).all() as Array<{ name: string }>
       const hasOwnerGateway = columns.some((c) => c.name === 'owner_gateway')
       if (!hasOwnerGateway) {
@@ -348,14 +354,27 @@ const migrations: Migration[] = [
         String(process.env.MC_DEFAULT_OWNER_GATEWAY || process.env.MC_DEFAULT_GATEWAY_NAME || 'primary').trim() ||
         'primary'
 
-      db.prepare(`
-        UPDATE tenants
-        SET owner_gateway = COALESCE(
-          (SELECT name FROM gateways ORDER BY is_primary DESC, id ASC LIMIT 1),
-          ?
-        )
-        WHERE owner_gateway IS NULL OR trim(owner_gateway) = ''
-      `).run(defaultGatewayName)
+      // Check if gateways table exists (created lazily by gateways API, not in migrations)
+      const hasGateways = (db.prepare(
+        `SELECT name FROM sqlite_master WHERE type='table' AND name='gateways'`
+      ).get() as any)
+
+      if (hasGateways) {
+        db.prepare(`
+          UPDATE tenants
+          SET owner_gateway = COALESCE(
+            (SELECT name FROM gateways ORDER BY is_primary DESC, id ASC LIMIT 1),
+            ?
+          )
+          WHERE owner_gateway IS NULL OR trim(owner_gateway) = ''
+        `).run(defaultGatewayName)
+      } else {
+        db.prepare(`
+          UPDATE tenants
+          SET owner_gateway = ?
+          WHERE owner_gateway IS NULL OR trim(owner_gateway) = ''
+        `).run(defaultGatewayName)
+      }
 
       db.exec(`CREATE INDEX IF NOT EXISTS idx_tenants_owner_gateway ON tenants(owner_gateway)`)
     }

tests/auth-guards.spec.ts

@@ -0,0 +1,45 @@
+import { test, expect } from '@playwright/test'
+
+/**
+ * E2E tests for Issue #4 — Auth guards on GET endpoints
+ * Verifies that unauthenticated requests to API endpoints are rejected.
+ */
+
+const PROTECTED_GET_ENDPOINTS = [
+  '/api/agents',
+  '/api/tasks',
+  '/api/activities',
+  '/api/notifications?recipient=test',
+  '/api/status',
+  '/api/logs',
+  '/api/chat/conversations',
+  '/api/chat/messages',
+  '/api/standup',
+  '/api/spawn',
+  '/api/pipelines',
+  '/api/pipelines/run',
+  '/api/webhooks',
+  '/api/webhooks/deliveries',
+  '/api/workflows',
+  '/api/settings',
+  '/api/tokens',
+  '/api/search?q=test',
+  '/api/audit',
+]
+
+test.describe('Auth Guards (Issue #4)', () => {
+  for (const endpoint of PROTECTED_GET_ENDPOINTS) {
+    test(`GET ${endpoint} returns 401 without auth`, async ({ request }) => {
+      const res = await request.get(endpoint)
+      expect(res.status()).toBe(401)
+    })
+  }
+
+  test('GET endpoint returns 200 with valid API key', async ({ request }) => {
+    const res = await request.get('/api/agents', {
+      headers: { 'x-api-key': 'test-api-key-e2e-12345' }
+    })
+    // Should be 200 (or possibly 500 if no gateway configured, but NOT 401)
+    expect(res.status()).not.toBe(401)
+  })
+})

tests/csrf-validation.spec.ts

@@ -0,0 +1,52 @@
+import { test, expect } from '@playwright/test'
+
+/**
+ * E2E tests for Issue #20 — CSRF Origin header validation
+ * Verifies that mutating requests with mismatched Origin are rejected.
+ */
+
+test.describe('CSRF Origin Validation (Issue #20)', () => {
+  test('POST with mismatched Origin is rejected', async ({ request }) => {
+    const res = await request.post('/api/auth/login', {
+      data: { username: 'test', password: 'test' },
+      headers: {
+        'origin': 'https://evil.example.com',
+        'host': '127.0.0.1:3005'
+      }
+    })
+    expect(res.status()).toBe(403)
+    const body = await res.json()
+    expect(body.error).toContain('CSRF')
+  })
+
+  test('POST with matching Origin is allowed', async ({ request }) => {
+    const res = await request.post('/api/auth/login', {
+      data: { username: 'testadmin', password: 'testpass123' },
+      headers: {
+        'origin': 'http://127.0.0.1:3005',
+        'host': '127.0.0.1:3005'
+      }
+    })
+    // Should not be 403 CSRF — may be 200 (success) or other status
+    expect(res.status()).not.toBe(403)
+  })
+
+  test('POST without Origin header is allowed (non-browser client)', async ({ request }) => {
+    const res = await request.post('/api/auth/login', {
+      data: { username: 'testadmin', password: 'testpass123' },
+    })
+    // No Origin = non-browser client, should be allowed through CSRF check
+    expect(res.status()).not.toBe(403)
+  })
+
+  test('GET requests are not subject to CSRF check', async ({ request }) => {
+    const res = await request.get('/api/agents', {
+      headers: {
+        'origin': 'https://evil.example.com',
+        'x-api-key': 'test-api-key-e2e-12345'
+      }
+    })
+    // GET is exempt from CSRF — should not be 403
+    expect(res.status()).not.toBe(403)
+  })
+})

tests/delete-body.spec.ts

@@ -0,0 +1,82 @@
+import { test, expect } from '@playwright/test'
+
+/**
+ * E2E tests for Issue #18 — DELETE handlers use request body
+ * Verifies that DELETE endpoints require JSON body instead of query params.
+ */
+
+const API_KEY_HEADER = { 'x-api-key': 'test-api-key-e2e-12345' }
+
+test.describe('DELETE Body Standardization (Issue #18)', () => {
+  test('DELETE /api/pipelines rejects without body', async ({ request }) => {
+    const res = await request.delete('/api/pipelines', {
+      headers: API_KEY_HEADER
+    })
+    const body = await res.json()
+    expect(body.error).toContain('body required')
+    expect(res.status()).toBe(400)
+  })
+
+  test('DELETE /api/pipelines accepts body with id', async ({ request }) => {
+    const res = await request.delete('/api/pipelines', {
+      headers: API_KEY_HEADER,
+      data: { id: '99999' }
+    })
+    // Should not be 400 "body required" — the body was provided
+    expect(res.status()).not.toBe(400)
+  })
+
+  test('DELETE /api/webhooks rejects without body', async ({ request }) => {
+    const res = await request.delete('/api/webhooks', {
+      headers: API_KEY_HEADER
+    })
+    const body = await res.json()
+    expect(body.error).toContain('body required')
+    expect(res.status()).toBe(400)
+  })
+
+  test('DELETE /api/settings rejects without body', async ({ request }) => {
+    const res = await request.delete('/api/settings', {
+      headers: API_KEY_HEADER
+    })
+    const body = await res.json()
+    expect(body.error).toContain('body required')
+    expect(res.status()).toBe(400)
+  })
+
+  test('DELETE /api/workflows rejects without body', async ({ request }) => {
+    const res = await request.delete('/api/workflows', {
+      headers: API_KEY_HEADER
+    })
+    const body = await res.json()
+    expect(body.error).toContain('body required')
+    expect(res.status()).toBe(400)
+  })
+
+  test('DELETE /api/backup rejects without body', async ({ request }) => {
+    const res = await request.delete('/api/backup', {
+      headers: API_KEY_HEADER
+    })
+    const body = await res.json()
+    expect(body.error).toContain('body required')
+    expect(res.status()).toBe(400)
+  })
+
+  test('DELETE /api/auth/users rejects without body', async ({ request }) => {
+    const res = await request.delete('/api/auth/users', {
+      headers: API_KEY_HEADER
+    })
+    const body = await res.json()
+    expect(body.error).toContain('body required')
+  })
+
+  test('old query param style no longer works for DELETE', async ({ request }) => {
+    // The old pattern: DELETE /api/pipelines?id=1
+    const res = await request.delete('/api/pipelines?id=1', {
+      headers: API_KEY_HEADER
+    })
+    // Without a JSON body, this should fail with "body required"
+    const body = await res.json()
+    expect(body.error).toContain('body required')
+  })
+})

tests/legacy-cookie-removed.spec.ts

@@ -0,0 +1,32 @@
+import { test, expect } from '@playwright/test'
+
+/**
+ * E2E tests for Issue #7 — Legacy cookie auth removal
+ * Verifies that the old mission-control-auth cookie no longer authenticates.
+ */
+
+test.describe('Legacy Cookie Auth Removed (Issue #7)', () => {
+  test('legacy cookie does not authenticate API requests', async ({ request }) => {
+    const res = await request.get('/api/agents', {
+      headers: {
+        'cookie': 'mission-control-auth=test-legacy-secret'
+      }
+    })
+    expect(res.status()).toBe(401)
+  })
+
+  test('legacy cookie does not authenticate page requests', async ({ page }) => {
+    // Set the legacy cookie
+    await page.context().addCookies([{
+      name: 'mission-control-auth',
+      value: 'test-legacy-secret',
+      domain: '127.0.0.1',
+      path: '/',
+    }])
+
+    // Try to access the main page — should redirect to login
+    const response = await page.goto('/')
+    const url = page.url()
+    expect(url).toContain('/login')
+  })
+})

tests/limit-caps.spec.ts

@@ -0,0 +1,54 @@
+import { test, expect } from '@playwright/test'
+
+/**
+ * E2E tests for Issue #19 — Unbounded limit caps
+ * Verifies that endpoints cap limit to 200 even if client requests more.
+ */
+
+const API_KEY_HEADER = { 'x-api-key': 'test-api-key-e2e-12345' }
+
+// These endpoints accept a `limit` query param
+const LIMIT_ENDPOINTS = [
+  '/api/agents',
+  '/api/tasks',
+  '/api/activities',
+  '/api/logs',
+  '/api/chat/conversations',
+  '/api/spawn',
+]
+
+test.describe('Limit Caps (Issue #19)', () => {
+  for (const endpoint of LIMIT_ENDPOINTS) {
+    test(`${endpoint}?limit=9999 does not return more than 200 items`, async ({ request }) => {
+      const res = await request.get(`${endpoint}?limit=9999`, {
+        headers: API_KEY_HEADER
+      })
+      // Should succeed (not error out)
+      expect(res.status()).not.toBe(500)
+
+      // The response should be valid JSON
+      const body = await res.json()
+      expect(body).toBeDefined()
+
+      // If the response has an array at the top level or nested, check its length
+      // Different endpoints return arrays under different keys
+      const possibleArrayKeys = ['agents', 'tasks', 'activities', 'logs', 'conversations', 'history', 'data']
+      for (const key of possibleArrayKeys) {
+        if (Array.isArray(body[key])) {
+          expect(body[key].length).toBeLessThanOrEqual(200)
+        }
+      }
+      // Also check if body itself is an array
+      if (Array.isArray(body)) {
+        expect(body.length).toBeLessThanOrEqual(200)
+      }
+    })
+  }
+
+  test('search endpoint has its own cap of 100', async ({ request }) => {
+    const res = await request.get('/api/search?q=test&limit=9999', {
+      headers: API_KEY_HEADER
+    })
+    expect(res.status()).not.toBe(500)
+  })
+})

tests/login-flow.spec.ts

@@ -0,0 +1,59 @@
+import { test, expect } from '@playwright/test'
+
+/**
+ * E2E smoke test — Login flow and session auth
+ * Verifies the basic login/session/logout lifecycle works end-to-end.
+ */
+
+test.describe('Login Flow', () => {
+  test('login page loads', async ({ page }) => {
+    await page.goto('/login')
+    await expect(page).toHaveURL(/\/login/)
+  })
+
+  test('unauthenticated access redirects to login', async ({ page }) => {
+    await page.goto('/')
+    await expect(page).toHaveURL(/\/login/)
+  })
+
+  test('login API returns session cookie on success', async ({ request }) => {
+    const res = await request.post('/api/auth/login', {
+      data: { username: 'testadmin', password: 'testpass123' }
+    })
+    expect(res.status()).toBe(200)
+
+    const cookies = res.headers()['set-cookie']
+    expect(cookies).toBeDefined()
+    expect(cookies).toContain('mc-session')
+  })
+
+  test('login API rejects wrong password', async ({ request }) => {
+    const res = await request.post('/api/auth/login', {
+      data: { username: 'testadmin', password: 'wrongpassword' },
+      headers: { 'x-forwarded-for': '10.77.77.77' }
+    })
+    expect(res.status()).toBe(401)
+  })
+
+  test('session cookie grants API access', async ({ request }) => {
+    // Login to get a session
+    const loginRes = await request.post('/api/auth/login', {
+      data: { username: 'testadmin', password: 'testpass123' }
+    })
+    expect(loginRes.status()).toBe(200)
+
+    // Extract session cookie from Set-Cookie header
+    const setCookie = loginRes.headers()['set-cookie'] || ''
+    const match = setCookie.match(/mc-session=([^;]+)/)
+    expect(match).toBeTruthy()
+    const sessionToken = match![1]
+
+    // Use the session cookie to access /api/auth/me
+    const meRes = await request.get('/api/auth/me', {
+      headers: { 'cookie': `mc-session=${sessionToken}` }
+    })
+    expect(meRes.status()).toBe(200)
+    const body = await meRes.json()
+    expect(body.user?.username).toBe('testadmin')
+  })
+})

tests/rate-limiting.spec.ts

@@ -0,0 +1,34 @@
+import { test, expect } from '@playwright/test'
+
+/**
+ * E2E tests for Issue #8 — Login rate limiting
+ * Verifies that login endpoint rate-limits after 5 failed attempts.
+ */
+
+test.describe('Login Rate Limiting (Issue #8)', () => {
+  test('blocks login after 5 rapid failed attempts', async ({ request }) => {
+    const results: number[] = []
+
+    // Send 7 rapid login attempts with wrong password
+    for (let i = 0; i < 7; i++) {
+      const res = await request.post('/api/auth/login', {
+        data: { username: 'testadmin', password: 'wrongpassword' },
+        headers: { 'x-forwarded-for': '10.99.99.99' }
+      })
+      results.push(res.status())
+    }
+
+    // First 5 should be 401 (wrong password), then 429 (rate limited)
+    const rateLimited = results.filter(s => s === 429)
+    expect(rateLimited.length).toBeGreaterThanOrEqual(1)
+  })
+
+  test('successful login is not blocked for fresh IP', async ({ request }) => {
+    const res = await request.post('/api/auth/login', {
+      data: { username: 'testadmin', password: 'testpass123' },
+      headers: { 'x-forwarded-for': '10.88.88.88' }
+    })
+    // Should succeed (200) or at least not be rate limited
+    expect(res.status()).not.toBe(429)
+  })
+})

tests/timing-safe-auth.spec.ts

@@ -0,0 +1,34 @@
+import { test, expect } from '@playwright/test'
+
+/**
+ * E2E tests for Issue #5 — Timing-safe API key comparison
+ * Verifies that API key auth works correctly after safeCompare migration.
+ */
+
+test.describe('Timing-Safe Auth (Issue #5)', () => {
+  test('valid API key authenticates successfully', async ({ request }) => {
+    const res = await request.get('/api/status', {
+      headers: { 'x-api-key': 'test-api-key-e2e-12345' }
+    })
+    expect(res.status()).not.toBe(401)
+  })
+
+  test('wrong API key is rejected', async ({ request }) => {
+    const res = await request.get('/api/status', {
+      headers: { 'x-api-key': 'wrong-key' }
+    })
+    expect(res.status()).toBe(401)
+  })
+
+  test('empty API key is rejected', async ({ request }) => {
+    const res = await request.get('/api/status', {
+      headers: { 'x-api-key': '' }
+    })
+    expect(res.status()).toBe(401)
+  })
+
+  test('no auth header is rejected', async ({ request }) => {
+    const res = await request.get('/api/status')
+    expect(res.status()).toBe(401)
+  })
+})