akanealw/Charon
1
0
Fork 0
Code Issues Pull Requests Actions 16 Packages Projects Releases Wiki Activity

fix: enhance security in account settings and notifications payload tests with API key masking and authorization headers

Browse Source
This commit is contained in:
GitHub Actions
2026-02-25 12:15:34 +00:00
parent 29f6664ab0
commit 7b1861f5a9
3 changed files with 99 additions and 65 deletions
Download Patch File Download Diff File Expand all files Collapse all files

63
docs/reports/qa_report.md
View File

@@ -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:

69
tests/settings/account-settings.spec.ts
View File

@@ -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);
});
});

32
tests/settings/notifications-payload.spec.ts
View File

@@ -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<string, unknown>;
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<string, unknown>;
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<string, unknown>;
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<string, unknown>;
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);