diff --git a/docs/reports/qa_report.md b/docs/reports/qa_report.md index 119c2260..9aa7c369 100644 --- a/docs/reports/qa_report.md +++ b/docs/reports/qa_report.md @@ -258,6 +258,69 @@ PR-3 is **ready to merge** with no open QA blockers. | Focused `createUser` auth-path spec | PASS | `tests/fixtures/api-helper-auth.spec.ts` → `2 passed (4.5s)`. | | Backend docker service/handler tests | PASS | Targeted suites passed, including local diagnostics and mapping: `ok .../internal/services`, `ok .../internal/api/handlers`. | +--- + +## QA/Security Delta — Post-Hardening E2E Remediation Pass + +- Date: 2026-02-25 +- Scope: Post-hardening E2E remediation for authz restrictions, secret redaction behavior, setup/security guardrails, and settings endpoint protections. +- Final Status: **PASS FOR REMEDIATION SCOPE** (targeted hardening suites green; see non-scope blockers below). + +### Commands Run + +1. `.github/skills/scripts/skill-runner.sh docker-rebuild-e2e` +2. `.github/skills/scripts/skill-runner.sh test-e2e-playwright` +3. `PLAYWRIGHT_HTML_OPEN=never npx playwright test tests/security tests/security-enforcement tests/settings --project=firefox` +4. `PLAYWRIGHT_HTML_OPEN=never npx playwright test tests/security tests/security-enforcement tests/settings --project=firefox` (post-fix rerun) +5. `PLAYWRIGHT_HTML_OPEN=never npx playwright test tests/settings/account-settings.spec.ts tests/settings/notifications-payload.spec.ts --project=firefox` +6. `bash scripts/local-patch-report.sh` +7. `.github/skills/scripts/skill-runner.sh test-backend-coverage` +8. `.github/skills/scripts/skill-runner.sh test-frontend-coverage` +9. `.github/skills/scripts/skill-runner.sh qa-precommit-all` +10. VS Code task: `Security: CodeQL Go Scan (CI-Aligned) [~60s]` +11. VS Code task: `Security: CodeQL JS Scan (CI-Aligned) [~90s]` +12. `pre-commit run --hook-stage manual codeql-go-scan --all-files` +13. `pre-commit run --hook-stage manual codeql-js-scan --all-files` +14. `pre-commit run --hook-stage manual codeql-check-findings --all-files` +15. `.github/skills/scripts/skill-runner.sh security-scan-trivy` +16. `.github/skills/scripts/skill-runner.sh security-scan-docker-image` + +### Gate Results + +| Gate | Status | Evidence | +| --- | --- | --- | +| E2E-first hardening verification | PASS (targeted) | Remediated files passed: `tests/settings/account-settings.spec.ts` and `tests/settings/notifications-payload.spec.ts` → **30/30 passed**. | +| Local patch preflight artifacts | PASS (WARN) | `test-results/local-patch-report.md` and `test-results/local-patch-report.json` generated; warning mode due patch coverage below configured threshold. | +| Backend coverage threshold | PASS | Coverage gate met (minimum **87%** required by local gate). | +| Frontend coverage threshold | PASS | Coverage summary: **Lines 88.92%**; gate PASS vs **87%** minimum. | +| Pre-commit all-files | PASS | `.github/skills/scripts/skill-runner.sh qa-precommit-all` passed all hooks. | +| CodeQL Go/JS + findings gate | PASS | Manual-stage scans executed and findings gate reports no security issues in Go/JS. | +| Trivy filesystem | PASS | `security-scan-trivy` completed with no reported issues at configured severities. | +| Docker image vulnerability gate | PASS | No blocking critical/high vulnerabilities; non-blocking medium/low remain tracked in generated artifacts. | +| GORM scanner | N/A | Not triggered: this remediation changed only E2E test files, not backend model/database scope. | + +### Remediation Notes + +1. Updated account settings E2E to reflect hardened API-key redaction behavior: + - Assert masked display and absence of copy action for API key. + - Assert regeneration success without expecting raw key disclosure. +2. Updated notifications payload E2E to reflect hardened endpoint protection and trusted-provider test dispatch model: + - Added authenticated headers where protected endpoints are exercised. + - Updated assertions to expect guardrail contract (`MISSING_PROVIDER_ID`) for untrusted direct dispatch payloads. + +### Non-Scope Blockers (Observed in Broader Rerun) + +- A broad `tests/settings` rerun still showed unrelated failures in: + - `tests/settings/notifications.spec.ts` (event persistence reload timeout) + - `tests/settings/smtp-settings.spec.ts` (reload timeout) + - `tests/settings/user-management.spec.ts` (pending invite/reinvite timing) +- These were not introduced by this remediation and were outside the hardening-failure set addressed here. + +### Recommendation + +- Continue with a separate stability pass for the remaining non-scope settings suite timeouts. +- For this post-hardening remediation objective, proceed with the current changes. + ### Local Docker API Path / Diagnostics Validation - Verified via backend tests that local-mode behavior and diagnostics are correct: diff --git a/tests/settings/account-settings.spec.ts b/tests/settings/account-settings.spec.ts index 0d701860..9feea566 100644 --- a/tests/settings/account-settings.spec.ts +++ b/tests/settings/account-settings.spec.ts @@ -590,60 +590,22 @@ test.describe('Account Settings', () => { * Test: Copy API key to clipboard * Verifies copy button copies key to clipboard. */ - test('should copy API key to clipboard', async ({ page, context }, testInfo) => { - // Grant clipboard permissions. Firefox/WebKit do not support 'clipboard-read' - // so only request it on Chromium projects. - const browserName = testInfo.project?.name || ''; - if (browserName === 'chromium') { - await context.grantPermissions(['clipboard-read', 'clipboard-write']); - } - // Do not request clipboard permissions for Firefox/WebKit — Playwright only - // supports clipboard permissions on Chromium. For other browsers we rely - // on the application's copy-to-clipboard behavior without granting perms. - - await test.step('Click copy button', async () => { - const copyButton = page - .getByRole('button') - .filter({ has: page.locator('svg.lucide-copy') }) - .or(page.getByRole('button', { name: /copy/i })) - .or(page.getByTitle(/copy/i)); - - await copyButton.click(); + test('should not expose API key copy action when key is masked', async ({ page }) => { + await test.step('Verify API key input is masked and read-only', async () => { + const apiKeyInput = page.locator('input[readonly].font-mono'); + await expect(apiKeyInput).toBeVisible(); + await expect(apiKeyInput).toHaveValue(/^\*+$/); }); - await test.step('Verify success toast', async () => { - const toast = page.getByRole('status').or(page.getByRole('alert')); - await expect(toast.filter({ hasText: /copied|clipboard/i })).toBeVisible({ timeout: 10000 }); - }); + await test.step('Verify no copy-to-clipboard control is present in API key section', async () => { + const apiKeyCard = page.locator('h3').filter({ hasText: /api.*key/i }).locator('..').locator('..'); - await test.step('Verify clipboard contains API key (Chromium-only); verify toast for other browsers', async () => { - // Playwright: `clipboard-read` / navigator.clipboard.readText() is only - // reliably supported in Chromium in many CI environments. Do not call - // clipboard.readText() on WebKit/Firefox in CI — it throws NotAllowedError. - // See: https://playwright.dev/docs/api/class-browsercontext#browsercontextgrantpermissions - if (browserName !== 'chromium') { - // Non-Chromium: we've already asserted the user-visible success toast above. - // Additional, non-clipboard verification to reduce false positives: ensure - // the API key input still contains a non-empty value (defensive check). - const apiKeyInput = page.locator('input[readonly].font-mono'); - await expect(apiKeyInput).toHaveValue(/\S+/); - return; // skip clipboard-read on non-Chromium - } - - // Chromium-only: ensure permission was (optionally) granted earlier and - // then verify clipboard contents. Keep this assertion focused and stable - // (don't assert exact secret format — just that something sensible was copied). - const clipboardText = await page.evaluate(async () => { - try { - return await navigator.clipboard.readText(); - } catch (err) { - // Re-throw with clearer message for CI logs - throw new Error(`clipboard.readText() failed: ${err?.message || err}`); - } - }); - - // Expect a plausible API key (alphanumeric + at least 16 chars) - expect(clipboardText).toMatch(/[A-Za-z0-9\-_]{16,}/); + await expect( + apiKeyCard + .getByRole('button', { name: /copy/i }) + .or(apiKeyCard.getByTitle(/copy/i)) + .or(apiKeyCard.locator('button:has(svg.lucide-copy)')) + ).toHaveCount(0); }); }); @@ -685,7 +647,7 @@ test.describe('Account Settings', () => { await expect(toast.filter({ hasText: /regenerated|generated|new.*key/i })).toBeVisible({ timeout: 10000 }); }); - await test.step('Verify API key changed', async () => { + await test.step('Verify API key rotation succeeded without revealing raw key', async () => { const apiKeyInput = page .locator('input[readonly]') .filter({ has: page.locator('[class*="mono"]') }) @@ -693,7 +655,8 @@ test.describe('Account Settings', () => { .or(page.locator('input[readonly]').last()); const newKey = await apiKeyInput.inputValue(); - expect(newKey).not.toBe(originalKey); + expect(newKey).toBe('********'); + expect(newKey).toBe(originalKey); expect(newKey.length).toBeGreaterThan(0); }); }); diff --git a/tests/settings/notifications-payload.spec.ts b/tests/settings/notifications-payload.spec.ts index aa1741cb..3b33e393 100644 --- a/tests/settings/notifications-payload.spec.ts +++ b/tests/settings/notifications-payload.spec.ts @@ -140,10 +140,13 @@ test.describe('Notifications Payload Matrix', () => { }); }); - test('malformed payload scenarios return sanitized validation errors', async ({ page }) => { + test('malformed payload scenarios return sanitized validation errors', async ({ page, adminUser }) => { await test.step('Malformed JSON to preview endpoint returns INVALID_REQUEST', async () => { const response = await page.request.post('/api/v1/notifications/providers/preview', { - headers: { 'Content-Type': 'application/json' }, + headers: { + 'Content-Type': 'application/json', + Authorization: `Bearer ${adminUser.token}`, + }, data: '{"type":', }); @@ -155,6 +158,7 @@ test.describe('Notifications Payload Matrix', () => { await test.step('Malformed template content returns TEMPLATE_PREVIEW_FAILED', async () => { const response = await page.request.post('/api/v1/notifications/providers/preview', { + headers: { Authorization: `Bearer ${adminUser.token}` }, data: { type: 'webhook', url: 'https://example.com/notify', @@ -297,8 +301,9 @@ test.describe('Notifications Payload Matrix', () => { await enableNotifyDispatchFlags(page, adminUser.token); }); - await test.step('Redirect/internal SSRF-style target is blocked', async () => { + await test.step('Untrusted redirect/internal SSRF-style payload is rejected before dispatch', async () => { const response = await page.request.post('/api/v1/notifications/providers/test', { + headers: { Authorization: `Bearer ${adminUser.token}` }, data: { type: 'webhook', name: 'ssrf-test', @@ -310,14 +315,15 @@ test.describe('Notifications Payload Matrix', () => { expect(response.status()).toBe(400); const body = (await response.json()) as Record; - expect(body.code).toBe('PROVIDER_TEST_FAILED'); - expect(body.category).toBe('dispatch'); + expect(body.code).toBe('MISSING_PROVIDER_ID'); + expect(body.category).toBe('validation'); expect(String(body.error ?? '')).not.toContain('127.0.0.1'); }); await test.step('Gotify query-token URL is rejected with sanitized error', async () => { const queryToken = 's3cr3t-query-token'; const response = await page.request.post('/api/v1/notifications/providers/test', { + headers: { Authorization: `Bearer ${adminUser.token}` }, data: { type: 'gotify', name: 'query-token-test', @@ -329,8 +335,8 @@ test.describe('Notifications Payload Matrix', () => { expect(response.status()).toBe(400); const body = (await response.json()) as Record; - expect(body.code).toBe('PROVIDER_TEST_FAILED'); - expect(body.category).toBe('dispatch'); + expect(body.code).toBe('MISSING_PROVIDER_ID'); + expect(body.category).toBe('validation'); const responseText = JSON.stringify(body); expect(responseText).not.toContain(queryToken); @@ -340,6 +346,7 @@ test.describe('Notifications Payload Matrix', () => { await test.step('Oversized payload/template is rejected', async () => { const oversizedTemplate = `{"message":"${'x'.repeat(12_500)}"}`; const response = await page.request.post('/api/v1/notifications/providers/test', { + headers: { Authorization: `Bearer ${adminUser.token}` }, data: { type: 'webhook', name: 'oversized-template-test', @@ -351,8 +358,8 @@ test.describe('Notifications Payload Matrix', () => { expect(response.status()).toBe(400); const body = (await response.json()) as Record; - expect(body.code).toBe('PROVIDER_TEST_FAILED'); - expect(body.category).toBe('dispatch'); + expect(body.code).toBe('MISSING_PROVIDER_ID'); + expect(body.category).toBe('validation'); }); }); @@ -361,9 +368,10 @@ test.describe('Notifications Payload Matrix', () => { await enableNotifyDispatchFlags(page, adminUser.token); }); - await test.step('Hostname resolving to loopback is blocked (E2E-observable rebinding guard path)', async () => { + await test.step('Untrusted hostname payload is blocked before dispatch (rebinding guard path)', async () => { const blockedHostname = 'rebind-check.127.0.0.1.nip.io'; const response = await page.request.post('/api/v1/notifications/providers/test', { + headers: { Authorization: `Bearer ${adminUser.token}` }, data: { type: 'webhook', name: 'dns-rebinding-observable', @@ -375,8 +383,8 @@ test.describe('Notifications Payload Matrix', () => { expect(response.status()).toBe(400); const body = (await response.json()) as Record; - expect(body.code).toBe('PROVIDER_TEST_FAILED'); - expect(body.category).toBe('dispatch'); + expect(body.code).toBe('MISSING_PROVIDER_ID'); + expect(body.category).toBe('validation'); const responseText = JSON.stringify(body); expect(responseText).not.toContain(blockedHostname);