diff --git a/.github/skills/test-e2e-playwright-scripts/run.sh b/.github/skills/test-e2e-playwright-scripts/run.sh new file mode 100755 index 00000000..395eac20 --- /dev/null +++ b/.github/skills/test-e2e-playwright-scripts/run.sh @@ -0,0 +1,188 @@ +#!/usr/bin/env bash +# Test E2E Playwright - Execution Script +# +# Runs Playwright end-to-end tests with browser selection, +# headed mode, and test filtering support. + +set -euo pipefail + +# Source helper scripts +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +# Helper scripts are in .github/skills/scripts/ (one level up from skill-scripts dir) +SKILLS_SCRIPTS_DIR="$(cd "${SCRIPT_DIR}/../scripts" && pwd)" + +# shellcheck source=../scripts/_logging_helpers.sh +source "${SKILLS_SCRIPTS_DIR}/_logging_helpers.sh" +# shellcheck source=../scripts/_error_handling_helpers.sh +source "${SKILLS_SCRIPTS_DIR}/_error_handling_helpers.sh" +# shellcheck source=../scripts/_environment_helpers.sh +source "${SKILLS_SCRIPTS_DIR}/_environment_helpers.sh" + +# Project root is 3 levels up from this script (skills/skill-name-scripts/run.sh -> project root) +PROJECT_ROOT="$(cd "${SCRIPT_DIR}/../../.." && pwd)" + +# Default parameter values +PROJECT="chromium" +HEADED=false +GREP="" + +# Parse command-line arguments +parse_arguments() { + while [[ $# -gt 0 ]]; do + case "$1" in + --project=*) + PROJECT="${1#*=}" + shift + ;; + --project) + PROJECT="${2:-chromium}" + shift 2 + ;; + --headed) + HEADED=true + shift + ;; + --grep=*) + GREP="${1#*=}" + shift + ;; + --grep) + GREP="${2:-}" + shift 2 + ;; + -h|--help) + show_help + exit 0 + ;; + *) + log_warning "Unknown argument: $1" + shift + ;; + esac + done +} + +# Show help message +show_help() { + cat << EOF +Usage: run.sh [OPTIONS] + +Run Playwright E2E tests against the Charon application. + +Options: + --project=PROJECT Browser project to run (chromium, firefox, webkit, all) + Default: chromium + --headed Run tests in headed mode (visible browser) + --grep=PATTERN Filter tests by title pattern (regex) + -h, --help Show this help message + +Environment Variables: + PLAYWRIGHT_BASE_URL Application URL to test (default: http://localhost:8080) + PLAYWRIGHT_HTML_OPEN HTML report behavior (default: never) + CI Set to 'true' for CI environment + +Examples: + run.sh # Run all tests in Chromium (headless) + run.sh --project=firefox # Run in Firefox + run.sh --headed # Run with visible browser + run.sh --grep="login" # Run only login tests + run.sh --project=all --grep="smoke" # All browsers, smoke tests only +EOF +} + +# Validate project parameter +validate_project() { + local valid_projects=("chromium" "firefox" "webkit" "all") + local project_lower + project_lower=$(echo "${PROJECT}" | tr '[:upper:]' '[:lower:]') + + for valid in "${valid_projects[@]}"; do + if [[ "${project_lower}" == "${valid}" ]]; then + PROJECT="${project_lower}" + return 0 + fi + done + + error_exit "Invalid project '${PROJECT}'. Valid options: chromium, firefox, webkit, all" +} + +# Build Playwright command arguments +build_playwright_args() { + local args=() + + # Add project selection + if [[ "${PROJECT}" != "all" ]]; then + args+=("--project=${PROJECT}") + fi + + # Add headed mode if requested + if [[ "${HEADED}" == "true" ]]; then + args+=("--headed") + fi + + # Add grep filter if specified + if [[ -n "${GREP}" ]]; then + args+=("--grep=${GREP}") + fi + + echo "${args[*]}" +} + +# Main execution +main() { + parse_arguments "$@" + + # Validate environment + log_step "ENVIRONMENT" "Validating prerequisites" + validate_node_environment "18.0" || error_exit "Node.js 18+ is required" + check_command_exists "npx" "npx is required (part of Node.js installation)" + + # Validate project structure + log_step "VALIDATION" "Checking project structure" + cd "${PROJECT_ROOT}" + validate_project_structure "tests" "playwright.config.js" "package.json" || error_exit "Invalid project structure" + + # Validate project parameter + validate_project + + # Set environment variables for non-interactive execution + export PLAYWRIGHT_HTML_OPEN="${PLAYWRIGHT_HTML_OPEN:-never}" + set_default_env "PLAYWRIGHT_BASE_URL" "http://localhost:8080" + + # Log configuration + log_step "CONFIG" "Test configuration" + log_info "Project: ${PROJECT}" + log_info "Headed mode: ${HEADED}" + log_info "Grep filter: ${GREP:-}" + log_info "Base URL: ${PLAYWRIGHT_BASE_URL}" + log_info "HTML report auto-open: ${PLAYWRIGHT_HTML_OPEN}" + + # Build command arguments + local playwright_args + playwright_args=$(build_playwright_args) + + # Execute Playwright tests + log_step "EXECUTION" "Running Playwright E2E tests" + log_command "npx playwright test ${playwright_args}" + + # Run tests with proper error handling + local exit_code=0 + # shellcheck disable=SC2086 + if npx playwright test ${playwright_args}; then + log_success "All E2E tests passed" + else + exit_code=$? + log_error "E2E tests failed (exit code: ${exit_code})" + fi + + # Output report location + log_step "REPORT" "Test report available" + log_info "HTML Report: ${PROJECT_ROOT}/playwright-report/index.html" + log_info "To view in browser: npx playwright show-report --port 9323" + log_info "VS Code Simple Browser URL: http://127.0.0.1:9323" + + exit "${exit_code}" +} + +# Run main with all arguments +main "$@" diff --git a/.github/skills/test-e2e-playwright.SKILL.md b/.github/skills/test-e2e-playwright.SKILL.md new file mode 100644 index 00000000..a0d35a10 --- /dev/null +++ b/.github/skills/test-e2e-playwright.SKILL.md @@ -0,0 +1,269 @@ +--- +# agentskills.io specification v1.0 +name: "test-e2e-playwright" +version: "1.0.0" +description: "Run Playwright E2E tests against the Charon application with browser selection and filtering" +author: "Charon Project" +license: "MIT" +tags: + - "testing" + - "e2e" + - "playwright" + - "integration" + - "browser" +compatibility: + os: + - "linux" + - "darwin" + shells: + - "bash" +requirements: + - name: "node" + version: ">=18.0" + optional: false + - name: "npx" + version: ">=1.0" + optional: false +environment_variables: + - name: "PLAYWRIGHT_BASE_URL" + description: "Base URL of the Charon application under test" + default: "http://localhost:8080" + required: false + - name: "PLAYWRIGHT_HTML_OPEN" + description: "Controls HTML report auto-open behavior (set to 'never' for CI/non-interactive)" + default: "never" + required: false + - name: "CI" + description: "Set to 'true' when running in CI environment" + default: "" + required: false +parameters: + - name: "project" + type: "string" + description: "Browser project to run (chromium, firefox, webkit, all)" + default: "chromium" + required: false + - name: "headed" + type: "boolean" + description: "Run tests in headed mode (visible browser)" + default: "false" + required: false + - name: "grep" + type: "string" + description: "Filter tests by title pattern (regex)" + default: "" + required: false +outputs: + - name: "playwright-report" + type: "directory" + description: "HTML test report directory" + path: "playwright-report/" + - name: "test-results" + type: "directory" + description: "Test artifacts and traces" + path: "test-results/" +metadata: + category: "test" + subcategory: "e2e" + execution_time: "medium" + risk_level: "low" + ci_cd_safe: true + requires_network: true + idempotent: true +--- + +# Test E2E Playwright + +## Overview + +Executes Playwright end-to-end tests against the Charon application. This skill supports browser selection, headed mode for debugging, and test filtering by name pattern. + +The skill runs non-interactively by default (HTML report does not auto-open), making it suitable for CI/CD pipelines and automated testing scenarios. + +## Prerequisites + +- Node.js 18.0 or higher installed and in PATH +- Playwright browsers installed (`npx playwright install`) +- Charon application running (default: `http://localhost:8080`) +- Test files in `tests/` directory + +## Usage + +### Basic Usage + +Run E2E tests with default settings (Chromium, headless): + +```bash +.github/skills/scripts/skill-runner.sh test-e2e-playwright +``` + +### Browser Selection + +Run tests in a specific browser: + +```bash +# Chromium (default) +.github/skills/scripts/skill-runner.sh test-e2e-playwright --project=chromium + +# Firefox +.github/skills/scripts/skill-runner.sh test-e2e-playwright --project=firefox + +# WebKit (Safari) +.github/skills/scripts/skill-runner.sh test-e2e-playwright --project=webkit + +# All browsers +.github/skills/scripts/skill-runner.sh test-e2e-playwright --project=all +``` + +### Headed Mode (Debugging) + +Run tests with a visible browser window: + +```bash +.github/skills/scripts/skill-runner.sh test-e2e-playwright --headed +``` + +### Filter Tests + +Run only tests matching a pattern: + +```bash +# Run tests with "login" in the title +.github/skills/scripts/skill-runner.sh test-e2e-playwright --grep="login" + +# Run tests with "DNS" in the title +.github/skills/scripts/skill-runner.sh test-e2e-playwright --grep="DNS" +``` + +### Combined Options + +```bash +.github/skills/scripts/skill-runner.sh test-e2e-playwright --project=firefox --headed --grep="dashboard" +``` + +### CI/CD Integration + +For use in GitHub Actions or other CI/CD pipelines: + +```yaml +- name: Run E2E Tests + run: .github/skills/scripts/skill-runner.sh test-e2e-playwright + env: + PLAYWRIGHT_BASE_URL: http://localhost:8080 + CI: true +``` + +## Parameters + +| Parameter | Type | Required | Default | Description | +|-----------|------|----------|---------|-------------| +| project | string | No | chromium | Browser project: chromium, firefox, webkit, all | +| headed | boolean | No | false | Run with visible browser window | +| grep | string | No | "" | Filter tests by title pattern (regex) | + +## Environment Variables + +| Variable | Required | Default | Description | +|----------|----------|---------|-------------| +| PLAYWRIGHT_BASE_URL | No | http://localhost:8080 | Application URL to test against | +| PLAYWRIGHT_HTML_OPEN | No | never | HTML report auto-open behavior | +| CI | No | "" | Set to "true" for CI environment behavior | + +## Outputs + +### Success Exit Code +- **0**: All tests passed + +### Error Exit Codes +- **1**: One or more tests failed +- **Non-zero**: Configuration or execution error + +### Output Directories +- **playwright-report/**: HTML report with test results and traces +- **test-results/**: Test artifacts, screenshots, and trace files + +## Viewing the Report + +After test execution, view the HTML report using VS Code Simple Browser: + +### Method 1: Start Report Server + +```bash +npx playwright show-report --port 9323 +``` + +Then open in VS Code Simple Browser: `http://127.0.0.1:9323` + +### Method 2: VS Code Task + +Use the VS Code task "Test: E2E Playwright - View Report" to start the report server as a background task, then open `http://127.0.0.1:9323` in Simple Browser. + +### Method 3: Direct File Access + +Open `playwright-report/index.html` directly in a browser. + +## Examples + +### Example 1: Quick Smoke Test + +```bash +.github/skills/scripts/skill-runner.sh test-e2e-playwright --grep="smoke" +``` + +### Example 2: Debug Failing Test + +```bash +.github/skills/scripts/skill-runner.sh test-e2e-playwright --headed --grep="failing-test-name" +``` + +### Example 3: Cross-Browser Validation + +```bash +.github/skills/scripts/skill-runner.sh test-e2e-playwright --project=all +``` + +## Test Structure + +Tests are located in the `tests/` directory and follow Playwright conventions: + +``` +tests/ +├── auth.setup.ts # Authentication setup (runs first) +├── dashboard.spec.ts # Dashboard tests +├── dns-records.spec.ts # DNS management tests +├── login.spec.ts # Login flow tests +└── ... +``` + +## Error Handling + +### Common Errors + +#### Error: Target page, context or browser has been closed +**Solution**: Ensure the application is running at the configured base URL + +#### Error: page.goto: net::ERR_CONNECTION_REFUSED +**Solution**: Start the Charon application before running tests + +#### Error: browserType.launch: Executable doesn't exist +**Solution**: Run `npx playwright install` to install browser binaries + +## Related Skills + +- test-frontend-unit - Frontend unit tests with Vitest +- docker-start-dev - Start development environment +- integration-test-all - Run all integration tests + +## Notes + +- **Authentication**: Tests use stored auth state from `playwright/.auth/user.json` +- **Parallelization**: Tests run in parallel locally, sequential in CI +- **Retries**: CI automatically retries failed tests twice +- **Traces**: Traces are collected on first retry for debugging +- **Report**: HTML report is generated at `playwright-report/index.html` + +--- + +**Last Updated**: 2026-01-15 +**Maintained by**: Charon Project Team +**Source**: `tests/` directory diff --git a/.vscode/tasks.json b/.vscode/tasks.json index 61e2f088..57ef31f8 100644 --- a/.vscode/tasks.json +++ b/.vscode/tasks.json @@ -391,6 +391,31 @@ "command": "echo '✅ Supply chain audit complete'", "group": "test", "problemMatcher": [] + }, + { + "label": "Test: E2E Playwright (Skill)", + "type": "shell", + "command": ".github/skills/scripts/skill-runner.sh test-e2e-playwright", + "group": "test", + "problemMatcher": [], + "presentation": { + "reveal": "always", + "panel": "dedicated", + "close": false + } + }, + { + "label": "Test: E2E Playwright - View Report", + "type": "shell", + "command": "npx playwright show-report --port 9323", + "group": "none", + "problemMatcher": [], + "isBackground": true, + "presentation": { + "reveal": "always", + "panel": "dedicated", + "close": false + } } ], "inputs": [ diff --git a/docs/plans/current_spec.md b/docs/plans/current_spec.md index a8c24514..17f59a73 100644 --- a/docs/plans/current_spec.md +++ b/docs/plans/current_spec.md @@ -1,851 +1,458 @@ +# Playwright E2E Skill with VS Code Simple Browser Integration -# Custom DNS Provider Plugin Support — Remaining Work Plan - -**Date**: 2026-01-14 - -This document is a phased completion plan for the remaining work on “Custom DNS Provider Plugin Support” on branch `feature/beta-release` (see PR #461 context in `CHANGELOG.md`). - -## What’s Already Implemented (Verified) - -- **Provider plugin registry**: `dnsprovider.Global()` registry and `dnsprovider.ProviderPlugin` interface in [backend/pkg/dnsprovider](backend/pkg/dnsprovider). -- **Built-in providers moved behind the registry**: 10 built-ins live in [backend/pkg/dnsprovider/builtin](backend/pkg/dnsprovider/builtin) and are registered via the blank import in [backend/cmd/api/main.go](backend/cmd/api/main.go). -- **External plugin loader**: `PluginLoaderService` in [backend/internal/services/plugin_loader.go](backend/internal/services/plugin_loader.go) (loads `.so`, validates metadata/interface version, optional SHA-256 allowlist, secure dir perms). -- **Plugin management backend** (Phase 5): admin endpoints in `backend/internal/api/handlers/plugin_handler.go` mounted under `/api/admin/plugins` via [backend/internal/api/routes/routes.go](backend/internal/api/routes/routes.go). -- **Example external plugin**: PowerDNS reference implementation in [plugins/powerdns](plugins/powerdns). -- **Registry-driven provider CRUD and Caddy config**: - - Provider validation/testing uses registry providers via [backend/internal/services/dns_provider_service.go](backend/internal/services/dns_provider_service.go) - - Caddy config generation is registry-driven (per Phase 5 docs) -- **Manual provider type**: `manual` provider plugin in [backend/pkg/dnsprovider/custom/manual_provider.go](backend/pkg/dnsprovider/custom/manual_provider.go). -- **Manual DNS challenge flow (UI + API)**: - - API handler: [backend/internal/api/handlers/manual_challenge_handler.go](backend/internal/api/handlers/manual_challenge_handler.go) - - Routes wired in [backend/internal/api/routes/routes.go](backend/internal/api/routes/routes.go) - - Frontend API/types: [frontend/src/api/manualChallenge.ts](frontend/src/api/manualChallenge.ts) - - Frontend UI: [frontend/src/components/dns-providers/ManualDNSChallenge.tsx](frontend/src/components/dns-providers/ManualDNSChallenge.tsx) -- **Playwright coverage exists** for manual provider flows: [tests/manual-dns-provider.spec.ts](tests/manual-dns-provider.spec.ts) - -## What’s Missing (Verified) - -- **Types endpoint is not registry-driven yet**: `GET /api/v1/dns-providers/types` is currently hardcoded in [backend/internal/api/handlers/dns_provider_handler.go](backend/internal/api/handlers/dns_provider_handler.go) and will not surface: - - the `manual` provider’s field specs - - any externally loaded plugin types (e.g., PowerDNS) - - any future custom providers registered in `dnsprovider.Global()` -- **Plugin signature allowlist is not wired**: `PluginLoaderService` supports an optional SHA-256 allowlist map, but [backend/cmd/api/main.go](backend/cmd/api/main.go) passes `nil`. -- **Sandboxing limitation is structural**: Go plugins run in-process (no OS sandbox). The only practical controls are deny-by-default plugin loading + allowlisting + secure deployment guidance. -- **No first-party webhook/script/rfc2136 provider types** exist as built-in `dnsprovider.ProviderPlugin` implementations (this is optional and should be treated as a separate feature, because external plugins already cover the extensibility goal). +**Date**: 2026-01-15 +**Status**: Planning +**Category**: Testing / Developer Experience --- -## Scope +## Overview -- Make DNS provider type discovery and UI configuration **registry-driven** so built-in + manual + externally loaded plugins show up correctly. -- Close the key security gap for external plugins by wiring an **operator-controlled allowlist** for plugin SHA-256 signatures. -- Keep the scope aligned to repo conventions: no Python, minimal new files, and follow the repository structure rules for any new docs. - -## Non-Goals - -- No Python scripts or example servers. -- No unrelated refactors of existing built-in providers. -- No “script execution provider” inside Charon (in-process shell execution is a separate high-risk feature and is explicitly out of scope here). -- No broad redesign of certificate issuance beyond what’s required for correct provider type discovery and safe plugin loading. - -## Dependencies - -- Backend provider registry: [backend/pkg/dnsprovider/plugin.go](backend/pkg/dnsprovider/plugin.go) -- Provider loader: [backend/internal/services/plugin_loader.go](backend/internal/services/plugin_loader.go) -- DNS provider UI/API type fetch: [frontend/src/api/dnsProviders.ts](frontend/src/api/dnsProviders.ts) -- Manual challenge API (used as a reference pattern for “non-Caddy” flows): [backend/internal/api/handlers/manual_challenge_handler.go](backend/internal/api/handlers/manual_challenge_handler.go) -- Container build pipeline: [Dockerfile](Dockerfile) (Caddy built via `xcaddy`) - -## Risks - -- **Type discovery mismatch**: UI uses `/api/v1/dns-providers/types`; if backend remains hardcoded, registry/manual/external plugin types won’t be configurable. -- **Supply-chain risk (plugins)**: `.so` loading is inherently sensitive; SHA-256 allowlist must be operator-controlled and deny-by-default in hardened deployments. -- **No sandbox**: Go plugins execute in-process with full memory access. Treat plugins as trusted code; document this clearly and avoid implying sandboxing. -- **SSRF / outbound calls**: plugins may implement `TestCredentials()` with outbound HTTP. Core cannot reliably enforce SSRF policy inside plugin code; mitigate via operational controls (restricted egress, allowlisted outbound via infra) and guidance for plugin authors to reuse Charon URL validators. -- **Patch coverage gate**: any production changes must maintain 100% patch coverage for modified lines. +Create a new agent skill that runs Playwright E2E tests non-interactively and opens the HTML report in VS Code's Simple Browser instead of launching an external browser. --- -## Definition of Done (DoD) Verification Gates (Per Phase) +## Analysis of Current Playwright Setup -Repository testing protocol requires Playwright E2E **before** unit tests. +### Configuration (`playwright.config.js`) -- **E2E (first)**: `npx playwright test --project=chromium` -- **Backend tests**: VS Code task `shell: Test: Backend with Coverage` -- **Frontend tests**: VS Code task `shell: Test: Frontend with Coverage` -- **TypeScript**: VS Code task `shell: Lint: TypeScript Check` -- **Pre-commit**: VS Code task `shell: Lint: Pre-commit (All Files)` -- **Security scans**: - - VS Code tasks `shell: Security: CodeQL Go Scan (CI-Aligned) [~60s]` and `shell: Security: CodeQL JS Scan (CI-Aligned) [~90s]` - - VS Code task `shell: Security: Trivy Scan` - - VS Code task `shell: Security: Go Vulnerability Check` +| Setting | Value | Notes | +|---------|-------|-------| +| Test Directory | `./tests` | All E2E tests reside here | +| Timeout | 30,000ms | Per-test timeout | +| Parallel | `fullyParallel: true` | Tests run in parallel locally | +| Retries | 0 (local), 2 (CI) | CI has retries enabled | +| Workers | unlimited (local), 1 (CI) | CI runs single worker | +| Base URL | `PLAYWRIGHT_BASE_URL` or `http://localhost:8080` | Configurable via env | +| Report Location | `playwright-report/` | HTML report output | -**Patch coverage requirement**: 100% for modified lines. +### Reporter Configuration ---- +```javascript +// CI mode +reporter: [['github'], ['html', { open: 'never' }]] -## Phase 1 — Registry-Driven Type Discovery (Unblocks UI + plugins) - -### Deliverables - -- Backend `GET /api/v1/dns-providers/types` returns **registry-driven** types, names, fields, and docs URLs. -- The types list includes: built-in providers, `manual`, and any external plugins loaded from `CHARON_PLUGINS_DIR`. -- Unit tests cover the new type discovery logic with 100% patch coverage on modified lines. - -### Tasks & Owners - -- **Backend_Dev** - - Replace hardcoded type list behavior in [backend/internal/api/handlers/dns_provider_handler.go](backend/internal/api/handlers/dns_provider_handler.go) with registry output. - - Use the service as the abstraction boundary: - - `h.service.GetSupportedProviderTypes()` for the type list - - `h.service.GetProviderCredentialFields(type)` for field specs - - `dnsprovider.Global().Get(type).Metadata()` for display name + docs URL - - Ensure the handler returns a stable, sorted list for predictable UI rendering. - - Add/adjust tests for the types endpoint. -- **Frontend_Dev** - - Confirm `getDNSProviderTypes()` is used as the single source of truth where appropriate. - - Keep the fallback schemas in `frontend/src/data/dnsProviderSchemas.ts` as a defensive measure, but prefer server-provided fields. -- **QA_Security** - - Validate that a newly registered provider type becomes visible in the UI without a frontend deploy. -- **Docs_Writer** - - Update operator docs explaining how types are surfaced and how plugins affect the UI. - -### Acceptance Criteria - -- Creating a `manual` provider is possible end-to-end using the types endpoint output. -- `/api/v1/dns-providers/types` includes `manual` and any externally loaded provider types (when present). -- 100% patch coverage for modified lines. - -### Verification Gates - -- If UI changed: run Playwright E2E first. -- Run backend + frontend coverage tasks, TypeScript check, pre-commit, and security scans. - ---- - -## Phase 2 — Provider Implementations: `rfc2136`, `webhook`, `script` - -This phase is **optional** and should only proceed if we explicitly want “first-party” provider types inside Charon (instead of shipping these as external `.so` plugins). External plugins already satisfy the extensibility goal. - -### Deliverables - -- New provider plugins implemented (as `dnsprovider.ProviderPlugin`): - - `rfc2136` - - `webhook` - - `script` -- Each provider defines: - - `Metadata()` (name/description/docs) - - `CredentialFields()` (field definitions for UI) - - Validation (required fields, value constraints) - - `BuildCaddyConfig()` (or explicit alternate flow) with deterministic JSON output - -### Tasks & Owners - -- **Backend_Dev** - - Add provider plugin files under [backend/pkg/dnsprovider/custom](backend/pkg/dnsprovider/custom) (pattern matches `manual_provider.go`). - - Define clear field schemas for each type (avoid guessing provider-specific parameters not supported by the underlying runtime; keep minimal + extensible). - - Implement validation errors that are actionable (which field, what’s wrong). - - Add unit tests for each provider plugin: - - metadata - - fields - - validation - - config generation -- **Frontend_Dev** - - Ensure provider forms render correctly from server-provided field definitions. - - Ensure any provider-specific help text uses the docs URL from the server type info. -- **Docs_Writer** - - Add/update docs pages for each provider type describing required fields and operational expectations. - -### Docker/Caddy Decision Checkpoint (Only if needed) - -Before changing Docker/Caddy: - -- Confirm whether the running Caddy build includes the required DNS modules for the new types. -- If a module is required and not present, update [Dockerfile](Dockerfile) `xcaddy build` arguments to include it. - -### Acceptance Criteria - -- `rfc2136`, `webhook`, and `script` show up in `/dns-providers/types` with complete field definitions. -- Creating and saving a provider of each type succeeds with validation. -- 100% patch coverage for modified lines. - -### Verification Gates - -- If UI changed: run Playwright E2E first. -- Run backend + frontend coverage tasks, TypeScript check, pre-commit, and security scans. - ---- - -## Phase 3 — Plugin Security Hardening & Operator Controls - -**Status**: ✅ **Implementation Complete, QA-Approved** (2026-01-14) -- Backend implementation complete -- QA security review passed -- Operator documentation published: [docs/features/plugin-security.md](docs/features/plugin-security.md) -- Remaining: Unit test coverage for `plugin_loader_test.go` - -### Current Implementation Analysis - -**PluginLoaderService Location**: [backend/internal/services/plugin_loader.go](backend/internal/services/plugin_loader.go) - -**Constructor Signature**: -```go -func NewPluginLoaderService(db *gorm.DB, pluginDir string, allowedSignatures map[string]string) *PluginLoaderService +// Local mode +reporter: [['list'], ['html', { open: 'on-failure' }]] ``` -**Service Struct**: -```go -type PluginLoaderService struct { - pluginDir string - allowedSigs map[string]string // plugin name (without .so) -> expected signature - loadedPlugins map[string]string // plugin type -> file path - db *gorm.DB - mu sync.RWMutex -} +**Key Finding**: The `open` option controls whether Playwright automatically opens the report: +- `'never'` - Never auto-open (used in CI) +- `'on-failure'` - Open only if tests fail (current local default) +- `'always'` - Always open after tests + +### Existing npm Scripts (`package.json`) + +| Script | Command | Description | +|--------|---------|-------------| +| `e2e` | `PLAYWRIGHT_HTML_OPEN=never npx playwright test --project=chromium` | Run Chromium only, no report open | +| `e2e:all` | `PLAYWRIGHT_HTML_OPEN=never npx playwright test` | Run all browsers, no report open | +| `e2e:headed` | `npx playwright test --project=chromium --headed` | Run headed (visible browser) | +| `e2e:report` | `npx playwright show-report` | **Opens report in default browser** | + +### Existing VS Code Tasks (`.vscode/tasks.json`) + +| Task Label | Command | Notes | +|------------|---------|-------| +| `Test: E2E Playwright (Chromium)` | `npm run e2e` | Non-interactive ✅ | +| `Test: E2E Playwright (All Browsers)` | `npm run e2e:all` | Non-interactive ✅ | +| `Test: E2E Playwright (Headed)` | `npm run e2e:headed` | Interactive/visual | + +**Gap Identified**: No task/skill exists to open the report in VS Code Simple Browser. + +### Current Skill Directory Structure + +Skills follow this pattern: +``` +.github/skills/ +├── {skill-name}.SKILL.md # Skill definition (frontmatter + docs) +├── {skill-name}-scripts/ +│ └── run.sh # Execution script ``` -**Existing Security Checks**: -1. `verifyDirectoryPermissions(dir)` — rejects world-writable directories (mode `0002`) -2. Signature verification in `LoadPlugin()` when `len(s.allowedSigs) > 0`: - - Checks if plugin name exists in allowlist → returns `dnsprovider.ErrPluginNotInAllowlist` if not - - Computes SHA-256 via `computeSignature()` → returns `dnsprovider.ErrSignatureMismatch` if different -3. Interface version check via `meta.InterfaceVersion` +--- -**Current main.go Usage** (line ~163): -```go -pluginLoader := services.NewPluginLoaderService(db, pluginDir, nil) // <-- nil bypasses allowlist +## Solution Design + +### Running Playwright Non-Interactively + +Playwright can be run without watching/waiting for report using: + +1. **Environment Variable**: `PLAYWRIGHT_HTML_OPEN=never` +2. **CLI Flag**: `--reporter=html` with no `--open` flag +3. **Config Override**: Already handled in `package.json` scripts + +The existing `npm run e2e` command already achieves this with `PLAYWRIGHT_HTML_OPEN=never`. + +### Opening HTML Report in VS Code Simple Browser + +VS Code provides the `simpleBrowser.show` command that can be invoked: + +1. **From Shell**: Using VS Code CLI: `code --goto file:///path/to/report` +2. **Using VS Code Command**: `simpleBrowser.api.open` or via tasks +3. **Using `open_simple_browser` tool**: For agent automation + +**Report Path**: `playwright-report/index.html` (relative to project root) + +### Skill Implementation Strategy + +Create a new skill `test-e2e-playwright` that: + +1. Runs Playwright tests non-interactively +2. Captures test results +3. Outputs the report path for VS Code integration +4. Optionally opens report in Simple Browser via VS Code command + +--- + +## Implementation Plan + +### Phase 1: Create Skill Files + +#### 1.1 Create Skill Definition File + +**File**: `.github/skills/test-e2e-playwright.SKILL.md` + +```yaml +--- +# agentskills.io specification v1.0 +name: "test-e2e-playwright" +version: "1.0.0" +description: "Run Playwright E2E tests non-interactively with HTML report generation" +author: "Charon Project" +license: "MIT" +tags: + - "testing" + - "e2e" + - "playwright" + - "integration" + - "browser" +compatibility: + os: + - "linux" + - "darwin" + shells: + - "bash" +requirements: + - name: "node" + version: ">=18.0" + optional: false + - name: "npx" + version: ">=8.0" + optional: false +environment_variables: + - name: "PLAYWRIGHT_BASE_URL" + description: "Base URL for tests (default: http://localhost:8080)" + required: false + - name: "PLAYWRIGHT_PROJECT" + description: "Browser project to run (chromium, firefox, webkit, or all)" + required: false +parameters: + - name: "project" + type: "string" + description: "Browser project (chromium, firefox, webkit, all)" + default: "chromium" + required: false + - name: "headed" + type: "boolean" + description: "Run in headed mode (visible browser)" + default: "false" + required: false + - name: "grep" + type: "string" + description: "Filter tests by title pattern" + required: false +outputs: + - name: "test_results" + type: "stdout" + description: "Playwright test output" + - name: "report_path" + type: "file" + description: "Path to HTML report (playwright-report/index.html)" +metadata: + category: "test" + subcategory: "e2e" + execution_time: "medium" + risk_level: "low" + ci_cd_safe: true + requires_network: true + idempotent: true +--- ``` -**Allowlist Behavior**: -- When `allowedSignatures` is `nil` or empty: all plugins are loaded (permissive mode) -- When `allowedSignatures` has entries: only listed plugins with matching signatures are allowed +#### 1.2 Create Execution Script -**Error Types** (from [backend/pkg/dnsprovider/errors.go](backend/pkg/dnsprovider/errors.go)): -- `dnsprovider.ErrPluginNotInAllowlist` — plugin name not found in allowlist map -- `dnsprovider.ErrSignatureMismatch` — SHA-256 hash doesn't match expected value +**File**: `.github/skills/test-e2e-playwright-scripts/run.sh` -### Design Decision: Option A (Env Var JSON Map) +```bash +#!/usr/bin/env bash +# Test E2E Playwright - Execution Script +# +# Runs Playwright E2E tests non-interactively and generates HTML report. -**Environment Variable**: `CHARON_PLUGIN_SIGNATURES` +set -euo pipefail -**Format**: JSON object mapping plugin filename (with `.so`) to SHA-256 signature -```json -{"powerdns.so": "sha256:abc123...", "myplugin.so": "sha256:def456..."} +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +SKILLS_SCRIPTS_DIR="$(cd "${SCRIPT_DIR}/../scripts" && pwd)" + +source "${SKILLS_SCRIPTS_DIR}/_logging_helpers.sh" +source "${SKILLS_SCRIPTS_DIR}/_error_handling_helpers.sh" +source "${SKILLS_SCRIPTS_DIR}/_environment_helpers.sh" + +PROJECT_ROOT="$(cd "${SCRIPT_DIR}/../../.." && pwd)" + +# Parse arguments +PROJECT="${PLAYWRIGHT_PROJECT:-chromium}" +HEADED="${PLAYWRIGHT_HEADED:-false}" +GREP_PATTERN="" + +while [[ $# -gt 0 ]]; do + case $1 in + --project=*) + PROJECT="${1#*=}" + shift + ;; + --headed) + HEADED="true" + shift + ;; + --grep=*) + GREP_PATTERN="${1#*=}" + shift + ;; + *) + break + ;; + esac +done + +# Validate environment +log_step "ENVIRONMENT" "Validating prerequisites" +validate_node_environment "18" || error_exit "Node.js 18+ is required" + +cd "${PROJECT_ROOT}" + +# Check Playwright is installed +if ! npx playwright --version &>/dev/null; then + log_error "Playwright not installed. Run: npm install" + exit 1 +fi + +# Build command +log_step "EXECUTION" "Running Playwright E2E tests" + +CMD_ARGS=("npx" "playwright" "test") + +# Add project filter +if [[ "${PROJECT}" != "all" ]]; then + CMD_ARGS+=("--project=${PROJECT}") +fi + +# Add headed mode +if [[ "${HEADED}" == "true" ]]; then + CMD_ARGS+=("--headed") +fi + +# Add grep pattern +if [[ -n "${GREP_PATTERN}" ]]; then + CMD_ARGS+=("--grep=${GREP_PATTERN}") +fi + +# Ensure report doesn't auto-open +export PLAYWRIGHT_HTML_OPEN=never + +log_info "Command: ${CMD_ARGS[*]}" +log_info "Base URL: ${PLAYWRIGHT_BASE_URL:-http://localhost:8080}" + +# Run tests +if "${CMD_ARGS[@]}" "$@"; then + EXIT_CODE=0 + log_success "Playwright E2E tests passed" +else + EXIT_CODE=$? + log_warning "Playwright E2E tests completed with failures (exit code: ${EXIT_CODE})" +fi + +# Output report path +REPORT_PATH="${PROJECT_ROOT}/playwright-report/index.html" +if [[ -f "${REPORT_PATH}" ]]; then + log_info "HTML Report: ${REPORT_PATH}" + echo "" + echo "━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━" + echo "📊 Report available at: file://${REPORT_PATH}" + echo "━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━" +fi + +exit "${EXIT_CODE}" ``` -**Behavior**: -| Env Var State | Behavior | -|---------------|----------| -| Unset/empty (`""`) | Permissive mode (backward compatible) — all plugins loaded | -| Set to `{}` | Strict mode with empty allowlist — no external plugins loaded | -| Set with entries | Strict mode — only listed plugins with matching signatures | +### Phase 2: Update VS Code Tasks -**Rationale for Option A**: -- Single env var keeps configuration surface minimal -- JSON is parseable in Go with `encoding/json` -- Follows existing pattern (`CHARON_PLUGINS_DIR`, `CHARON_CROWDSEC_*`) -- Operators can generate signatures with: `sha256sum plugin.so | awk '{print "sha256:" $1}'` +Add new tasks to `.vscode/tasks.json`: -### Deliverables - -1. **Parse and wire allowlist in main.go** -2. **Helper function to parse signature env var** -3. **Unit tests for PluginLoaderService** (currently missing!) -4. **Operator documentation** - -### Implementation Tasks - -#### Task 3.1: Add Signature Parsing Helper - -**File**: [backend/cmd/api/main.go](backend/cmd/api/main.go) (or new file `backend/internal/config/plugin_config.go`) - -```go -// parsePluginSignatures parses the CHARON_PLUGIN_SIGNATURES env var. -// Returns nil if unset/empty (permissive mode). -// Returns empty map if set to "{}" (strict mode, no plugins). -// Returns populated map if valid JSON with entries. -func parsePluginSignatures() (map[string]string, error) { - raw := os.Getenv("CHARON_PLUGIN_SIGNATURES") - if raw == "" { - return nil, nil // Permissive mode +```jsonc +{ + "label": "Test: E2E Playwright (Skill)", + "type": "shell", + "command": ".github/skills/scripts/skill-runner.sh test-e2e-playwright", + "group": "test", + "problemMatcher": [], + "presentation": { + "reveal": "always", + "panel": "dedicated", + "close": false } - - var sigs map[string]string - if err := json.Unmarshal([]byte(raw), &sigs); err != nil { - return nil, fmt.Errorf("invalid CHARON_PLUGIN_SIGNATURES JSON: %w", err) +}, +{ + "label": "Test: E2E Playwright - View Report", + "type": "shell", + "command": "echo 'Opening report...'", + "group": "test", + "problemMatcher": [], + "runOptions": { + "runOn": "default" + }, + "presentation": { + "reveal": "never" } - return sigs, nil } ``` -#### Task 3.2: Wire Parsing into main.go +### Phase 3: VS Code Simple Browser Integration -**File**: [backend/cmd/api/main.go](backend/cmd/api/main.go) +The VS Code Simple Browser can be opened via: -**Change** (around line 163): -```go -// Before: -pluginLoader := services.NewPluginLoaderService(db, pluginDir, nil) +#### Option A: VS Code Command (Preferred) -// After: -pluginSignatures, err := parsePluginSignatures() -if err != nil { - log.Fatalf("parse plugin signatures: %v", err) -} -if pluginSignatures != nil { - logger.Log().Infof("Plugin signature allowlist enabled with %d entries", len(pluginSignatures)) -} else { - logger.Log().Info("Plugin signature allowlist not configured (permissive mode)") -} -pluginLoader := services.NewPluginLoaderService(db, pluginDir, pluginSignatures) +Create a compound task that: +1. Runs tests +2. Opens Simple Browser with report + +**Implementation**: Use `vscode.commands.executeCommand('simpleBrowser.show', uri)` from an extension or the `simpleBrowser.api.open` command. + +For agent usage, the `open_simple_browser` tool can be used: + +``` +URL: file:///projects/Charon/playwright-report/index.html ``` -#### Task 3.3: Create PluginLoaderService Unit Tests +#### Option B: HTTP Server Approach -**File**: [backend/internal/services/plugin_loader_test.go](backend/internal/services/plugin_loader_test.go) (NEW) +Start a local HTTP server to serve the report: -**Test Scenarios**: +```bash +npx playwright show-report --host 127.0.0.1 --port 9323 +``` -| Test Name | Setup | Expected Result | -|-----------|-------|-----------------| -| `TestNewPluginLoaderService_NilAllowlist` | `allowedSignatures: nil` | Service created, `allowedSigs` is nil | -| `TestNewPluginLoaderService_EmptyAllowlist` | `allowedSignatures: map[string]string{}` | Service created, `allowedSigs` is empty map | -| `TestNewPluginLoaderService_PopulatedAllowlist` | `allowedSignatures: {"test.so": "sha256:abc"}` | Service created with entries | -| `TestLoadPlugin_AllowlistEmpty_SkipsVerification` | Empty allowlist, mock plugin | Plugin loads without signature check | -| `TestLoadPlugin_AllowlistSet_PluginNotListed` | Allowlist without plugin | Returns `ErrPluginNotInAllowlist` | -| `TestLoadPlugin_AllowlistSet_SignatureMismatch` | Allowlist with wrong hash | Returns `ErrSignatureMismatch` | -| `TestLoadPlugin_AllowlistSet_SignatureMatch` | Allowlist with correct hash | Plugin loads successfully | -| `TestVerifyDirectoryPermissions_Secure` | Dir mode `0755` | Returns nil | -| `TestVerifyDirectoryPermissions_WorldWritable` | Dir mode `0777` | Returns error | -| `TestComputeSignature_ValidFile` | Real file | Returns `sha256:...` string | -| `TestLoadAllPlugins_DirectoryNotExist` | Non-existent dir | Returns nil (graceful skip) | -| `TestLoadAllPlugins_DirectoryInsecure` | World-writable dir | Returns error | +Then open in Simple Browser: `http://127.0.0.1:9323` -**Note**: Testing actual `.so` loading requires CGO and platform-specific binaries. Focus unit tests on: -- Constructor behavior -- `verifyDirectoryPermissions()` (create temp dirs) -- `computeSignature()` (create temp files) -- Allowlist logic flow (mock the actual `plugin.Open` call) +**Note**: This requires the server to stay running. -#### Task 3.4: Create parsePluginSignatures Unit Tests +#### Option C: File Protocol -**File**: [backend/cmd/api/main_test.go](backend/cmd/api/main_test.go) or integrate into plugin_loader_test.go +Open directly as file URL (simplest): -| Test Name | Env Value | Expected Result | -|-----------|-----------|-----------------| -| `TestParsePluginSignatures_Unset` | (not set) | `nil, nil` | -| `TestParsePluginSignatures_Empty` | `""` | `nil, nil` | -| `TestParsePluginSignatures_EmptyObject` | `"{}"` | `map[string]string{}, nil` | -| `TestParsePluginSignatures_Valid` | `{"a.so":"sha256:x"}` | `map with entry, nil` | -| `TestParsePluginSignatures_InvalidJSON` | `"not json"` | `nil, error` | -| `TestParsePluginSignatures_MultipleEntries` | `{"a.so":"sha256:x","b.so":"sha256:y"}` | `map with 2 entries, nil` | - -### Tasks & Owners - -- **Backend_Dev** - - [x] Create `parsePluginSignatures()` helper function ✅ *Completed 2026-01-14* - - [x] Update [backend/cmd/api/main.go](backend/cmd/api/main.go) to wire parsed signatures ✅ *Completed 2026-01-14* - - [ ] Create [backend/internal/services/plugin_loader_test.go](backend/internal/services/plugin_loader_test.go) with comprehensive test coverage - - [x] Add logging for allowlist mode (enabled vs permissive) ✅ *Completed 2026-01-14* -- **DevOps** - - [x] Ensure the plugin directory is mounted read-only in production (`/app/plugins:ro`) ✅ *Completed 2026-01-14* - - [x] Validate container permissions align with `verifyDirectoryPermissions()` (mode `0755` or stricter) ✅ *Completed 2026-01-14* - - [x] Document how to generate plugin signatures: `sha256sum plugin.so | awk '{print "sha256:" $1}'` ✅ *See below* -- **QA_Security** - - [x] Threat model review focused on `.so` loading risks ✅ *QA-approved 2026-01-14* - - [x] Verify error messages don't leak sensitive path information ✅ *QA-approved 2026-01-14* - - [x] Test edge cases: symlinks, race conditions, permission changes ✅ *QA-approved 2026-01-14* -- **Docs_Writer** - - [x] Create/update plugin operator docs explaining: ✅ *Completed 2026-01-14* - - `CHARON_PLUGIN_SIGNATURES` format and behavior - - How to compute signatures - - Recommended deployment pattern (read-only mounts, strict allowlist) - - Security implications of permissive mode - - [x] Created [docs/features/plugin-security.md](docs/features/plugin-security.md) ✅ *Completed 2026-01-14* - -### Acceptance Criteria - -- [x] Plugins load successfully when signature matches allowlist ✅ *QA-approved* -- [x] Plugins are rejected with `ErrPluginNotInAllowlist` when not in allowlist ✅ *QA-approved* -- [x] Plugins are rejected with `ErrSignatureMismatch` when hash differs ✅ *QA-approved* -- [x] World-writable plugin directory is detected and prevents all plugin loading ✅ *QA-approved* -- [x] Empty/unset `CHARON_PLUGIN_SIGNATURES` maintains backward compatibility (permissive) ✅ *QA-approved* -- [x] Invalid JSON in `CHARON_PLUGIN_SIGNATURES` causes startup failure with clear error ✅ *QA-approved* -- [ ] 100% patch coverage for modified lines in `main.go` -- [ ] New `plugin_loader_test.go` achieves high coverage of testable code paths -- [x] Operator documentation created: [docs/features/plugin-security.md](docs/features/plugin-security.md) ✅ *Completed 2026-01-14* - -### Verification Gates - -- Run backend coverage task: `shell: Test: Backend with Coverage` -- Run security scans: - - `shell: Security: CodeQL Go Scan (CI-Aligned) [~60s]` - - `shell: Security: Go Vulnerability Check` -- Run pre-commit: `shell: Lint: Pre-commit (All Files)` - -### Risks & Mitigations - -| Risk | Mitigation | -|------|------------| -| Invalid JSON crashes startup | Explicit error handling with descriptive message | -| Plugin name mismatch (with/without `.so`) | Document exact format; code expects filename as key | -| Signature format confusion | Enforce `sha256:` prefix; reject malformed signatures | -| Race condition: plugin modified after signature check | Document atomic deployment pattern (copy then rename) | -| Operators forget to update signatures after plugin update | Log warning when signature verification is enabled | +``` +file:///projects/Charon/playwright-report/index.html +``` --- -## Phase 4 — E2E Coverage + Regression Safety +## Files to Create/Modify -**Status**: ✅ **Implementation Complete** (2026-01-15) -- 55 tests created across 3 test files -- All tests passing (52 pass, 3 conditional skip) -- Test files: `dns-provider-types.spec.ts`, `dns-provider-crud.spec.ts`, `manual-dns-provider.spec.ts` -- Fixtures created: `tests/fixtures/dns-providers.ts` +### New Files -### Current Test Coverage Analysis +| File | Purpose | +|------|---------| +| `.github/skills/test-e2e-playwright.SKILL.md` | Skill definition | +| `.github/skills/test-e2e-playwright-scripts/run.sh` | Execution script | -**Existing Test Files**: -| File | Purpose | Coverage Status | -|------|---------|-----------------| -| `tests/example.spec.js` | Playwright example (external site) | Not relevant to Charon | -| `tests/manual-dns-provider.spec.ts` | Manual DNS provider E2E tests | Good foundation, many tests skipped | +### Modified Files -**Existing `manual-dns-provider.spec.ts` Coverage**: -- ✅ Provider Selection Flow (navigation tests) -- ✅ Manual Challenge UI Display (conditional tests) -- ✅ Copy to Clipboard functionality -- ✅ Verify Button Interactions -- ✅ Accessibility Checks (keyboard navigation, ARIA) -- ✅ Component Tests (mocked API responses) -- ✅ Error Handling tests - -**Gaps Identified**: -1. **Types Endpoint Not Tested**: No tests verify `/api/v1/dns-providers/types` returns all provider types (built-in + custom + plugins) -2. **Provider Creation Flows**: No E2E tests for creating providers of each type -3. **Provider List Rendering**: No tests verify the provider cards grid renders correctly -4. **Edit/Delete Provider Flows**: No coverage for provider management operations -5. **Form Field Validation**: No tests for required field validation errors -6. **Dynamic Field Rendering**: No tests verify fields render from server-provided definitions -7. **Plugin Provider Types**: No tests for external plugin types (e.g., `powerdns`) - -### Deliverables - -1. **New Test File**: `tests/dns-provider-types.spec.ts` — Types endpoint and selector rendering -2. **New Test File**: `tests/dns-provider-crud.spec.ts` — Provider creation, edit, delete flows -3. **Updated Test File**: `tests/manual-dns-provider.spec.ts` — Enable skipped tests, add missing coverage -4. **Operator Smoke Test Documentation**: `docs/testing/e2e-smoke-tests.md` - -### Test File Organization - -``` -tests/ -├── example.spec.js # (Keep as Playwright reference) -├── manual-dns-provider.spec.ts # (Existing - Manual DNS challenge flow) -├── dns-provider-types.spec.ts # (NEW - Provider types endpoint & selector) -├── dns-provider-crud.spec.ts # (NEW - CRUD operations & validation) -└── dns-provider-a11y.spec.ts # (NEW - Focused accessibility tests) -``` - -### Test Scenarios (Prioritized) - -#### Priority 1: Core Functionality (Must Pass Before Merge) - -**File: `dns-provider-types.spec.ts`** - -| Test Name | Description | API Verified | -|-----------|-------------|--------------| -| `GET /dns-providers/types returns all built-in providers` | Verify cloudflare, route53, digitalocean, etc. in response | `GET /api/v1/dns-providers/types` | -| `GET /dns-providers/types includes custom providers` | Verify manual, webhook, rfc2136, script in response | `GET /api/v1/dns-providers/types` | -| `Provider selector dropdown shows all types` | Verify dropdown options match API response | UI + API | -| `Provider selector groups by category` | Built-in vs custom categorization | UI | -| `Provider type selection updates form fields` | Changing type loads correct credential fields | UI | - -**File: `dns-provider-crud.spec.ts`** - -| Test Name | Description | API Verified | -|-----------|-------------|--------------| -| `Create Cloudflare provider with valid credentials` | Complete create flow for built-in type | `POST /api/v1/dns-providers` | -| `Create Manual provider successfully` | Complete create flow for custom type | `POST /api/v1/dns-providers` | -| `Form shows validation errors for missing required fields` | Submit without required fields shows errors | UI validation | -| `Test Connection button shows success/failure` | Pre-save credential validation | `POST /api/v1/dns-providers/test` | -| `Edit provider updates name and settings` | Modify existing provider | `PUT /api/v1/dns-providers/:id` | -| `Delete provider with confirmation` | Delete flow with modal | `DELETE /api/v1/dns-providers/:id` | -| `Provider list renders all providers as cards` | Grid layout verification | `GET /api/v1/dns-providers` | - -#### Priority 2: Regression Safety (Manual DNS Challenge) - -**File: `manual-dns-provider.spec.ts`** (Enable and Update) - -| Test Name | Status | Action Required | -|-----------|--------|-----------------| -| `should navigate to DNS Providers page` | ✅ Active | Keep | -| `should show Add Provider button on DNS Providers page` | ⏭️ Skipped | **Enable** - requires backend | -| `should display Manual option in provider selection` | ⏭️ Skipped | **Enable** - requires backend | -| `should display challenge panel with required elements` | ✅ Conditional | Add mock data fixture | -| `Copy to clipboard functionality` | ✅ Conditional | Add fixture | -| `Verify button interactions` | ✅ Conditional | Add fixture | -| `Accessibility checks` | ✅ Partial | Expand coverage | - -**New Tests for Manual Flow**: -| Test Name | Description | -|-----------|-------------| -| `Create manual provider and verify in list` | Full create → list → verify flow | -| `Manual provider shows "Pending Challenge" state` | Verify UI state when challenge is active | -| `Manual challenge countdown timer decrements` | Time remaining updates correctly | -| `Manual challenge verification completes flow` | Success path when DNS propagates | - -#### Priority 3: Accessibility Compliance - -**File: `dns-provider-a11y.spec.ts`** - -| Test Name | WCAG Criteria | -|-----------|---------------| -| `Provider form has properly associated labels` | 1.3.1 Info and Relationships | -| `Error messages are announced to screen readers` | 4.1.3 Status Messages | -| `Keyboard navigation through form fields` | 2.1.1 Keyboard | -| `Focus visible on all interactive elements` | 2.4.7 Focus Visible | -| `Password fields are not autocompleted` | Security best practice | -| `Dialog trap focus correctly` | 2.4.3 Focus Order | -| `Form submission button has loading state` | 4.1.2 Name, Role, Value | - -#### Priority 4: Plugin Provider Types (Optional - When Plugins Present) - -**File: `dns-provider-crud.spec.ts`** (Conditional Tests) - -| Test Name | Condition | -|-----------|-----------| -| `External plugin types appear in selector` | `CHARON_PLUGINS_DIR` has `.so` files | -| `Create provider for plugin type (e.g., powerdns)` | Plugin type available in API | -| `Plugin provider test connection works` | Plugin credentials valid | - -### Implementation Guidance - -#### Test Data Strategy - -```typescript -// tests/fixtures/dns-providers.ts -export const mockProviderTypes = { - built_in: ['cloudflare', 'route53', 'digitalocean', 'googleclouddns'], - custom: ['manual', 'webhook', 'rfc2136', 'script'], -} - -export const mockCloudflareProvider = { - name: 'Test Cloudflare', - provider_type: 'cloudflare', - credentials: { - api_token: 'test-token-12345', - }, -} - -export const mockManualProvider = { - name: 'Test Manual', - provider_type: 'manual', - credentials: {}, -} -``` - -#### API Mocking Pattern (From Existing Tests) - -```typescript -// Mock provider types endpoint -await page.route('**/api/v1/dns-providers/types', async (route) => { - await route.fulfill({ - status: 200, - contentType: 'application/json', - body: JSON.stringify({ - types: [ - { type: 'cloudflare', name: 'Cloudflare', fields: [...] }, - { type: 'manual', name: 'Manual DNS', fields: [] }, - ], - }), - }); -}); -``` - -#### Test Structure Pattern (Following Existing Conventions) - -```typescript -import { test, expect } from '@playwright/test'; - -const BASE_URL = process.env.PLAYWRIGHT_BASE_URL || 'http://localhost:3003'; - -test.describe('DNS Provider Types', () => { - test.beforeEach(async ({ page }) => { - await page.goto(BASE_URL); - }); - - test('should display all provider types in selector', async ({ page }) => { - await test.step('Navigate to DNS Providers', async () => { - await page.goto(`${BASE_URL}/dns-providers`); - }); - - await test.step('Open Add Provider dialog', async () => { - await page.getByRole('button', { name: /add provider/i }).click(); - }); - - await test.step('Verify provider type options', async () => { - const providerSelect = page.getByRole('combobox', { name: /provider type/i }); - await providerSelect.click(); - - // Verify built-in providers - await expect(page.getByRole('option', { name: /cloudflare/i })).toBeVisible(); - await expect(page.getByRole('option', { name: /route53/i })).toBeVisible(); - - // Verify custom providers - await expect(page.getByRole('option', { name: /manual/i })).toBeVisible(); - }); - }); -}); -``` - -### Tasks & Owners - -- **QA_Security** - - [ ] Create `tests/dns-provider-types.spec.ts` with Priority 1 type tests - - [ ] Create `tests/dns-provider-crud.spec.ts` with Priority 1 CRUD tests - - [ ] Enable skipped tests in `tests/manual-dns-provider.spec.ts` - - [ ] Create `tests/dns-provider-a11y.spec.ts` with Priority 3 accessibility tests - - [ ] Create `tests/fixtures/dns-providers.ts` with mock data - - [ ] Document smoke test procedures in `docs/testing/e2e-smoke-tests.md` -- **Frontend_Dev** - - [ ] Fix any UI issues uncovered by E2E (focus order, error announcements, labels) - - [ ] Ensure form field IDs are stable for test selectors - - [ ] Add `data-testid` attributes where role-based selectors are insufficient -- **Backend_Dev** - - [ ] Fix any API contract mismatches discovered by E2E - - [ ] Ensure `/api/v1/dns-providers/types` returns complete field definitions - - [ ] Verify error response format matches frontend expectations - -### Potential Issues to Watch - -Based on code analysis, these may cause test failures (fix code first, per user directive): - -| Potential Issue | Component | Symptom | -|-----------------|-----------|---------| -| Types endpoint hardcoded | `dns_provider_handler.go` | Manual/plugin types missing from selector | -| Missing field definitions | API response | Form renders without credential fields | -| Dialog not trapping focus | `DNSProviderForm.tsx` | Tab escapes dialog | -| Select not keyboard accessible | `ui/Select.tsx` | Cannot navigate with arrow keys | -| Toast not announced | `toast.ts` | Screen readers miss success/error messages | - -### Acceptance Criteria - -- [ ] All Priority 1 tests pass reliably in Chromium -- [ ] All Priority 2 (manual provider regression) tests pass -- [ ] No skipped tests in `manual-dns-provider.spec.ts` (except documented exclusions) -- [ ] Priority 3 accessibility tests pass (or issues documented for fix) -- [ ] Smoke test documentation complete and validated by QA - -### Verification Gates - -1. **Run Playwright E2E first**: `npx playwright test --project=chromium` -2. **If tests fail**: Analyze whether failure is test bug or application bug - - Application bug → Fix code first, then re-run tests - - Test bug → Fix test, document reasoning -3. **After E2E passes**: Run full verification suite - - Backend coverage: `shell: Test: Backend with Coverage` - - Frontend coverage: `shell: Test: Frontend with Coverage` - - TypeScript check: `shell: Lint: TypeScript Check` - - Pre-commit: `shell: Lint: Pre-commit (All Files)` - - Security scans: CodeQL + Trivy + Go Vulnerability Check +| File | Changes | +|------|---------| +| `.vscode/tasks.json` | Add new Playwright skill task | --- -## Phase 5 — Test Coverage Gaps (Required Before Merge) +## Usage Examples -**Status**: ✅ **Complete** (2026-01-15) +### Basic Usage (Agent) -### Context +```bash +# Run E2E tests +.github/skills/scripts/skill-runner.sh test-e2e-playwright -DoD verification passed overall (85%+ coverage), but specific gaps were identified during Issue #21 / PR #461 completeness review. +# Then open report in VS Code Simple Browser +# (Agent can use open_simple_browser tool with file:///projects/Charon/playwright-report/index.html) +``` -### Deliverables +### Specific Browser -1. **Unit tests for `plugin_loader.go`** — ✅ Comprehensive tests already exist -2. **Cover missing line in `encryption_handler.go`** — ✅ Documented as defensive error handling, added tests -3. **Enable skipped E2E tests** — ✅ Validated full integration +```bash +.github/skills/scripts/skill-runner.sh test-e2e-playwright --project=firefox +``` -### Tasks & Owners +### Headed Mode (Debugging) -#### Task 5.1: Create `plugin_loader_test.go` +```bash +.github/skills/scripts/skill-runner.sh test-e2e-playwright --headed +``` -**File**: [backend/internal/services/plugin_loader_test.go](backend/internal/services/plugin_loader_test.go) +### Filter by Test Name -**Status**: ✅ **Complete** — Tests already exist with comprehensive coverage - -- **Backend_Dev** - - [x] `TestNewPluginLoaderService_NilAllowlist` — ✅ Exists as `TestNewPluginLoaderServicePermissiveMode` - - [x] `TestNewPluginLoaderService_EmptyAllowlist` — ✅ Exists as `TestNewPluginLoaderServiceStrictModeEmpty` - - [x] `TestNewPluginLoaderService_PopulatedAllowlist` — ✅ Exists as `TestNewPluginLoaderServiceStrictModePopulated` - - [x] `TestVerifyDirectoryPermissions_Secure` — ✅ Exists as `TestVerifyDirectoryPermissions` (mode 0755) - - [x] `TestVerifyDirectoryPermissions_WorldWritable` — ✅ Exists as `TestVerifyDirectoryPermissions` (mode 0777) - - [x] `TestComputeSignature_ValidFile` — ✅ Exists as `TestComputeSignature` - - [x] `TestLoadAllPlugins_DirectoryNotExist` — ✅ Exists as `TestLoadAllPluginsNonExistentDirectory` - - [x] `TestLoadAllPlugins_DirectoryInsecure` — ✅ Exists as `TestLoadAllPluginsWorldWritableDirectory` - -**Additional tests found in existing file:** -- `TestComputeSignatureNonExistentFile` -- `TestComputeSignatureConsistency` -- `TestComputeSignatureLargeFile` -- `TestComputeSignatureSpecialCharactersInPath` -- `TestLoadPluginNotInAllowlist` -- `TestLoadPluginSignatureMismatch` -- `TestLoadPluginSignatureMatch` -- `TestLoadPluginPermissiveMode` -- `TestLoadAllPluginsEmptyDirectory` -- `TestLoadAllPluginsEmptyPluginDir` -- `TestLoadAllPluginsSkipsDirectories` -- `TestLoadAllPluginsSkipsNonSoFiles` -- `TestListLoadedPluginsEmpty` -- `TestIsPluginLoadedFalse` -- `TestUnloadNonExistentPlugin` -- `TestCleanupEmpty` -- `TestParsePluginSignaturesLogic` -- `TestSignatureWorkflowEndToEnd` -- `TestGenerateUUIDUniqueness` -- `TestGenerateUUIDFormat` -- `TestConcurrentPluginMapAccess` - -**Note**: Actual `.so` loading requires CGO and is platform-specific. Tests focus on testable paths: -- Constructor behavior -- `verifyDirectoryPermissions()` with temp directories -- `computeSignature()` with temp files -- Allowlist validation logic - -#### Task 5.2: Cover `encryption_handler.go` Missing Line - -**Status**: ✅ **Complete** — Added documentation tests, identified defensive error handling - -- **Backend_Dev** - - [x] Identify uncovered line (likely error path in decrypt/encrypt flow) - - **Finding**: Lines 162-179 (`Validate` error path) require `ValidateKeyConfiguration()` to fail - - **Root Cause**: This only fails if `rs.currentKey == nil` (impossible after successful service creation) - - **Conclusion**: This is defensive error handling; cannot be triggered without mocking - - [x] Add targeted test case to reach 100% patch coverage - - Added `TestEncryptionHandler_Rotate_AuditChannelFull` — Tests audit channel saturation scenario - - Added `TestEncryptionHandler_Validate_ValidationFailurePath` — Documents the untestable path - -**Tests Added** (in `encryption_handler_test.go`): -- `TestEncryptionHandler_Rotate_AuditChannelFull` — Covers audit logging edge case -- `TestEncryptionHandler_Validate_ValidationFailurePath` — Documents limitation - -**Coverage Analysis**: -- `Validate` function at 60% — The uncovered 40% is defensive error handling -- `Rotate` function at 92.9% — Audit start log failure (line 63) is also defensive -- These paths exist for robustness but cannot be triggered in production without internal state corruption - -#### Task 5.3: Enable Skipped E2E Tests - -**Status**: ✅ **Previously Complete** (Phase 4) - -- **QA_Security** - - [x] Review skipped tests in `tests/manual-dns-provider.spec.ts` - - [x] Enable tests that have backend support - - [x] Document any tests that remain skipped with rationale - -### Acceptance Criteria - -- [x] `plugin_loader_test.go` exists with comprehensive coverage ✅ -- [x] 100% patch coverage for modified lines in PR #461 ✅ (Defensive paths documented) -- [x] All E2E tests enabled (or documented exclusions) ✅ -- [x] All verification gates pass ✅ - -### Verification Gates (Completed) - -- [x] Backend coverage: All plugin loader and encryption handler tests pass -- [x] E2E tests: Previously completed in Phase 4 -- [x] Pre-commit: No new lint errors introduced +```bash +.github/skills/scripts/skill-runner.sh test-e2e-playwright --grep="login" +``` --- -## Phase 6 — User Documentation (Recommended) +## Integration with Agent Workflow -**Status**: ✅ **Complete** (2026-01-15) +When an agent needs to run E2E tests and view results: -### Context - -Core functionality is complete. User-facing documentation has been updated to reflect the new DNS Challenge feature. - -### Completed -- ✅ Rewrote `docs/features.md` as marketing overview (249 lines, down from 1,952 — 87% reduction) -- ✅ Added DNS Challenge feature to features.md with provider list and key benefits -- ✅ Organized features into 8 logical categories with "Learn More" links -- ✅ Created comprehensive `docs/features/dns-challenge.md` (DNS Challenge documentation) -- ✅ Created 18 feature stub pages for documentation consistency -- ✅ Updated README.md to include DNS Challenge in Top Features - -### Deliverables - -1. ~~**Rewrite `docs/features.md`**~~ — ✅ Complete (marketing overview style per new guidelines) -2. ~~**DNS Challenge Feature Docs**~~ — ✅ Complete (`docs/features/dns-challenge.md`) -3. ~~**Feature Stub Pages**~~ — ✅ Complete (18 stubs created) -4. ~~**Update README**~~ — ✅ Complete (DNS Challenge added to Top Features) - -### Tasks & Owners - -#### Task 6.1: Create DNS Troubleshooting Guide - -**File**: [docs/features/dns-troubleshooting.md](docs/features/dns-troubleshooting.md) (NEW) - -- **Docs_Writer** - - [ ] Common issues section: - - DNS propagation delays (TTL) - - Incorrect API credentials - - Missing permissions (e.g., Zone:Edit for Cloudflare) - - Firewall blocking outbound DNS API calls - - [ ] Verification steps: - - How to check if TXT record exists: `dig TXT _acme-challenge.example.com` - - How to verify credentials work before certificate request - - [ ] Provider-specific gotchas: - - Cloudflare: Zone ID vs API Token scopes - - Route53: IAM policy requirements - - DigitalOcean: API token permissions - -#### Task 6.2: Create Provider Quick-Setup Guides - -**Files**: -- [docs/providers/cloudflare.md](docs/providers/cloudflare.md) (NEW) -- [docs/providers/route53.md](docs/providers/route53.md) (NEW) -- [docs/providers/digitalocean.md](docs/providers/digitalocean.md) (NEW) - -- **Docs_Writer** - - [ ] Step-by-step credential creation (with screenshots/links) - - [ ] Required permissions/scopes - - [ ] Example Charon configuration - - [ ] Testing the provider connection - -#### Task 6.3: Update README Feature List - -**File**: [README.md](README.md) - -- **Docs_Writer** - - [ ] Add DNS Challenge / Wildcard Certificates to feature list - - [ ] Link to detailed documentation - -### Acceptance Criteria - -- [ ] DNS troubleshooting guide covers top 5 common issues -- [ ] At least 3 provider quick-setup guides exist -- [ ] README mentions wildcard certificate support -- [ ] Documentation follows markdown lint rules - -### Verification Gates - -- Run markdown lint: `npm run lint:md` -- Manual review of documentation accuracy +1. **Run skill**: `skill-runner.sh test-e2e-playwright` +2. **Check exit code**: 0 = pass, non-zero = failures +3. **Open report**: Use `open_simple_browser` tool with URL: + - File URL: `file:///projects/Charon/playwright-report/index.html` + - Or start server and use: `http://127.0.0.1:9323` --- -## Open Questions (Need Explicit Decisions) +## Acceptance Criteria -- ~~For plugin signature allowlisting: what is the desired configuration shape?~~ - - **DECIDED: Option A (minimal)**: env var `CHARON_PLUGIN_SIGNATURES` with JSON map `pluginFilename.so` → `sha256:...` parsed by [backend/cmd/api/main.go](backend/cmd/api/main.go). See Phase 3 for full specification. - - ~~**Option B (operator-friendly)**: load from a mounted file path (adds new config surface)~~ — Not chosen; JSON env var is sufficient and simpler. -- For “first-party” providers (`webhook`, `script`, `rfc2136`): are these still required given external plugins already exist? +- [ ] Skill runs Playwright tests without interactive report watcher +- [ ] HTML report is generated at `playwright-report/index.html` +- [ ] Skill outputs report path for easy consumption +- [ ] Exit code reflects test pass/fail status +- [ ] Report can be opened in VS Code Simple Browser +- [ ] Supports all Playwright projects (chromium, firefox, webkit) +- [ ] Supports headed mode for debugging +- [ ] Supports grep filtering for specific tests --- -## Notes on Accessibility +## Notes -UI work in this plan is built with accessibility in mind, but likely still requires manual review and testing (e.g., with Accessibility Insights) as changes land. +1. **File Protocol Limitation**: Some browsers restrict `file://` protocol. VS Code Simple Browser handles this well. + +2. **Report Serving**: For interactive debugging, `npx playwright show-report` starts a server but blocks. The skill approach avoids this. + +3. **CI/CD Safety**: The skill is CI/CD safe since it doesn't depend on UI interaction. + +4. **Base URL**: Tests use `PLAYWRIGHT_BASE_URL` environment variable, defaulting to `http://localhost:8080`. + +--- + +## Related Documentation + +- [Playwright Test Configuration](https://playwright.dev/docs/test-configuration) +- [VS Code Simple Browser](https://code.visualstudio.com/docs/editor/extension-marketplace#_simple-browser) +- [Agent Skills Specification](https://agentskills.io/specification) +- [Existing Playwright Tasks](.vscode/tasks.json) + +--- + +**Last Updated**: 2026-01-15 +**Author**: GitHub Copilot +**Status**: Ready for Implementation diff --git a/docs/plans/current_spec.md.backup_playwright_skill b/docs/plans/current_spec.md.backup_playwright_skill new file mode 100644 index 00000000..a8c24514 --- /dev/null +++ b/docs/plans/current_spec.md.backup_playwright_skill @@ -0,0 +1,851 @@ + +# Custom DNS Provider Plugin Support — Remaining Work Plan + +**Date**: 2026-01-14 + +This document is a phased completion plan for the remaining work on “Custom DNS Provider Plugin Support” on branch `feature/beta-release` (see PR #461 context in `CHANGELOG.md`). + +## What’s Already Implemented (Verified) + +- **Provider plugin registry**: `dnsprovider.Global()` registry and `dnsprovider.ProviderPlugin` interface in [backend/pkg/dnsprovider](backend/pkg/dnsprovider). +- **Built-in providers moved behind the registry**: 10 built-ins live in [backend/pkg/dnsprovider/builtin](backend/pkg/dnsprovider/builtin) and are registered via the blank import in [backend/cmd/api/main.go](backend/cmd/api/main.go). +- **External plugin loader**: `PluginLoaderService` in [backend/internal/services/plugin_loader.go](backend/internal/services/plugin_loader.go) (loads `.so`, validates metadata/interface version, optional SHA-256 allowlist, secure dir perms). +- **Plugin management backend** (Phase 5): admin endpoints in `backend/internal/api/handlers/plugin_handler.go` mounted under `/api/admin/plugins` via [backend/internal/api/routes/routes.go](backend/internal/api/routes/routes.go). +- **Example external plugin**: PowerDNS reference implementation in [plugins/powerdns](plugins/powerdns). +- **Registry-driven provider CRUD and Caddy config**: + - Provider validation/testing uses registry providers via [backend/internal/services/dns_provider_service.go](backend/internal/services/dns_provider_service.go) + - Caddy config generation is registry-driven (per Phase 5 docs) +- **Manual provider type**: `manual` provider plugin in [backend/pkg/dnsprovider/custom/manual_provider.go](backend/pkg/dnsprovider/custom/manual_provider.go). +- **Manual DNS challenge flow (UI + API)**: + - API handler: [backend/internal/api/handlers/manual_challenge_handler.go](backend/internal/api/handlers/manual_challenge_handler.go) + - Routes wired in [backend/internal/api/routes/routes.go](backend/internal/api/routes/routes.go) + - Frontend API/types: [frontend/src/api/manualChallenge.ts](frontend/src/api/manualChallenge.ts) + - Frontend UI: [frontend/src/components/dns-providers/ManualDNSChallenge.tsx](frontend/src/components/dns-providers/ManualDNSChallenge.tsx) +- **Playwright coverage exists** for manual provider flows: [tests/manual-dns-provider.spec.ts](tests/manual-dns-provider.spec.ts) + +## What’s Missing (Verified) + +- **Types endpoint is not registry-driven yet**: `GET /api/v1/dns-providers/types` is currently hardcoded in [backend/internal/api/handlers/dns_provider_handler.go](backend/internal/api/handlers/dns_provider_handler.go) and will not surface: + - the `manual` provider’s field specs + - any externally loaded plugin types (e.g., PowerDNS) + - any future custom providers registered in `dnsprovider.Global()` +- **Plugin signature allowlist is not wired**: `PluginLoaderService` supports an optional SHA-256 allowlist map, but [backend/cmd/api/main.go](backend/cmd/api/main.go) passes `nil`. +- **Sandboxing limitation is structural**: Go plugins run in-process (no OS sandbox). The only practical controls are deny-by-default plugin loading + allowlisting + secure deployment guidance. +- **No first-party webhook/script/rfc2136 provider types** exist as built-in `dnsprovider.ProviderPlugin` implementations (this is optional and should be treated as a separate feature, because external plugins already cover the extensibility goal). + +--- + +## Scope + +- Make DNS provider type discovery and UI configuration **registry-driven** so built-in + manual + externally loaded plugins show up correctly. +- Close the key security gap for external plugins by wiring an **operator-controlled allowlist** for plugin SHA-256 signatures. +- Keep the scope aligned to repo conventions: no Python, minimal new files, and follow the repository structure rules for any new docs. + +## Non-Goals + +- No Python scripts or example servers. +- No unrelated refactors of existing built-in providers. +- No “script execution provider” inside Charon (in-process shell execution is a separate high-risk feature and is explicitly out of scope here). +- No broad redesign of certificate issuance beyond what’s required for correct provider type discovery and safe plugin loading. + +## Dependencies + +- Backend provider registry: [backend/pkg/dnsprovider/plugin.go](backend/pkg/dnsprovider/plugin.go) +- Provider loader: [backend/internal/services/plugin_loader.go](backend/internal/services/plugin_loader.go) +- DNS provider UI/API type fetch: [frontend/src/api/dnsProviders.ts](frontend/src/api/dnsProviders.ts) +- Manual challenge API (used as a reference pattern for “non-Caddy” flows): [backend/internal/api/handlers/manual_challenge_handler.go](backend/internal/api/handlers/manual_challenge_handler.go) +- Container build pipeline: [Dockerfile](Dockerfile) (Caddy built via `xcaddy`) + +## Risks + +- **Type discovery mismatch**: UI uses `/api/v1/dns-providers/types`; if backend remains hardcoded, registry/manual/external plugin types won’t be configurable. +- **Supply-chain risk (plugins)**: `.so` loading is inherently sensitive; SHA-256 allowlist must be operator-controlled and deny-by-default in hardened deployments. +- **No sandbox**: Go plugins execute in-process with full memory access. Treat plugins as trusted code; document this clearly and avoid implying sandboxing. +- **SSRF / outbound calls**: plugins may implement `TestCredentials()` with outbound HTTP. Core cannot reliably enforce SSRF policy inside plugin code; mitigate via operational controls (restricted egress, allowlisted outbound via infra) and guidance for plugin authors to reuse Charon URL validators. +- **Patch coverage gate**: any production changes must maintain 100% patch coverage for modified lines. + +--- + +## Definition of Done (DoD) Verification Gates (Per Phase) + +Repository testing protocol requires Playwright E2E **before** unit tests. + +- **E2E (first)**: `npx playwright test --project=chromium` +- **Backend tests**: VS Code task `shell: Test: Backend with Coverage` +- **Frontend tests**: VS Code task `shell: Test: Frontend with Coverage` +- **TypeScript**: VS Code task `shell: Lint: TypeScript Check` +- **Pre-commit**: VS Code task `shell: Lint: Pre-commit (All Files)` +- **Security scans**: + - VS Code tasks `shell: Security: CodeQL Go Scan (CI-Aligned) [~60s]` and `shell: Security: CodeQL JS Scan (CI-Aligned) [~90s]` + - VS Code task `shell: Security: Trivy Scan` + - VS Code task `shell: Security: Go Vulnerability Check` + +**Patch coverage requirement**: 100% for modified lines. + +--- + +## Phase 1 — Registry-Driven Type Discovery (Unblocks UI + plugins) + +### Deliverables + +- Backend `GET /api/v1/dns-providers/types` returns **registry-driven** types, names, fields, and docs URLs. +- The types list includes: built-in providers, `manual`, and any external plugins loaded from `CHARON_PLUGINS_DIR`. +- Unit tests cover the new type discovery logic with 100% patch coverage on modified lines. + +### Tasks & Owners + +- **Backend_Dev** + - Replace hardcoded type list behavior in [backend/internal/api/handlers/dns_provider_handler.go](backend/internal/api/handlers/dns_provider_handler.go) with registry output. + - Use the service as the abstraction boundary: + - `h.service.GetSupportedProviderTypes()` for the type list + - `h.service.GetProviderCredentialFields(type)` for field specs + - `dnsprovider.Global().Get(type).Metadata()` for display name + docs URL + - Ensure the handler returns a stable, sorted list for predictable UI rendering. + - Add/adjust tests for the types endpoint. +- **Frontend_Dev** + - Confirm `getDNSProviderTypes()` is used as the single source of truth where appropriate. + - Keep the fallback schemas in `frontend/src/data/dnsProviderSchemas.ts` as a defensive measure, but prefer server-provided fields. +- **QA_Security** + - Validate that a newly registered provider type becomes visible in the UI without a frontend deploy. +- **Docs_Writer** + - Update operator docs explaining how types are surfaced and how plugins affect the UI. + +### Acceptance Criteria + +- Creating a `manual` provider is possible end-to-end using the types endpoint output. +- `/api/v1/dns-providers/types` includes `manual` and any externally loaded provider types (when present). +- 100% patch coverage for modified lines. + +### Verification Gates + +- If UI changed: run Playwright E2E first. +- Run backend + frontend coverage tasks, TypeScript check, pre-commit, and security scans. + +--- + +## Phase 2 — Provider Implementations: `rfc2136`, `webhook`, `script` + +This phase is **optional** and should only proceed if we explicitly want “first-party” provider types inside Charon (instead of shipping these as external `.so` plugins). External plugins already satisfy the extensibility goal. + +### Deliverables + +- New provider plugins implemented (as `dnsprovider.ProviderPlugin`): + - `rfc2136` + - `webhook` + - `script` +- Each provider defines: + - `Metadata()` (name/description/docs) + - `CredentialFields()` (field definitions for UI) + - Validation (required fields, value constraints) + - `BuildCaddyConfig()` (or explicit alternate flow) with deterministic JSON output + +### Tasks & Owners + +- **Backend_Dev** + - Add provider plugin files under [backend/pkg/dnsprovider/custom](backend/pkg/dnsprovider/custom) (pattern matches `manual_provider.go`). + - Define clear field schemas for each type (avoid guessing provider-specific parameters not supported by the underlying runtime; keep minimal + extensible). + - Implement validation errors that are actionable (which field, what’s wrong). + - Add unit tests for each provider plugin: + - metadata + - fields + - validation + - config generation +- **Frontend_Dev** + - Ensure provider forms render correctly from server-provided field definitions. + - Ensure any provider-specific help text uses the docs URL from the server type info. +- **Docs_Writer** + - Add/update docs pages for each provider type describing required fields and operational expectations. + +### Docker/Caddy Decision Checkpoint (Only if needed) + +Before changing Docker/Caddy: + +- Confirm whether the running Caddy build includes the required DNS modules for the new types. +- If a module is required and not present, update [Dockerfile](Dockerfile) `xcaddy build` arguments to include it. + +### Acceptance Criteria + +- `rfc2136`, `webhook`, and `script` show up in `/dns-providers/types` with complete field definitions. +- Creating and saving a provider of each type succeeds with validation. +- 100% patch coverage for modified lines. + +### Verification Gates + +- If UI changed: run Playwright E2E first. +- Run backend + frontend coverage tasks, TypeScript check, pre-commit, and security scans. + +--- + +## Phase 3 — Plugin Security Hardening & Operator Controls + +**Status**: ✅ **Implementation Complete, QA-Approved** (2026-01-14) +- Backend implementation complete +- QA security review passed +- Operator documentation published: [docs/features/plugin-security.md](docs/features/plugin-security.md) +- Remaining: Unit test coverage for `plugin_loader_test.go` + +### Current Implementation Analysis + +**PluginLoaderService Location**: [backend/internal/services/plugin_loader.go](backend/internal/services/plugin_loader.go) + +**Constructor Signature**: +```go +func NewPluginLoaderService(db *gorm.DB, pluginDir string, allowedSignatures map[string]string) *PluginLoaderService +``` + +**Service Struct**: +```go +type PluginLoaderService struct { + pluginDir string + allowedSigs map[string]string // plugin name (without .so) -> expected signature + loadedPlugins map[string]string // plugin type -> file path + db *gorm.DB + mu sync.RWMutex +} +``` + +**Existing Security Checks**: +1. `verifyDirectoryPermissions(dir)` — rejects world-writable directories (mode `0002`) +2. Signature verification in `LoadPlugin()` when `len(s.allowedSigs) > 0`: + - Checks if plugin name exists in allowlist → returns `dnsprovider.ErrPluginNotInAllowlist` if not + - Computes SHA-256 via `computeSignature()` → returns `dnsprovider.ErrSignatureMismatch` if different +3. Interface version check via `meta.InterfaceVersion` + +**Current main.go Usage** (line ~163): +```go +pluginLoader := services.NewPluginLoaderService(db, pluginDir, nil) // <-- nil bypasses allowlist +``` + +**Allowlist Behavior**: +- When `allowedSignatures` is `nil` or empty: all plugins are loaded (permissive mode) +- When `allowedSignatures` has entries: only listed plugins with matching signatures are allowed + +**Error Types** (from [backend/pkg/dnsprovider/errors.go](backend/pkg/dnsprovider/errors.go)): +- `dnsprovider.ErrPluginNotInAllowlist` — plugin name not found in allowlist map +- `dnsprovider.ErrSignatureMismatch` — SHA-256 hash doesn't match expected value + +### Design Decision: Option A (Env Var JSON Map) + +**Environment Variable**: `CHARON_PLUGIN_SIGNATURES` + +**Format**: JSON object mapping plugin filename (with `.so`) to SHA-256 signature +```json +{"powerdns.so": "sha256:abc123...", "myplugin.so": "sha256:def456..."} +``` + +**Behavior**: +| Env Var State | Behavior | +|---------------|----------| +| Unset/empty (`""`) | Permissive mode (backward compatible) — all plugins loaded | +| Set to `{}` | Strict mode with empty allowlist — no external plugins loaded | +| Set with entries | Strict mode — only listed plugins with matching signatures | + +**Rationale for Option A**: +- Single env var keeps configuration surface minimal +- JSON is parseable in Go with `encoding/json` +- Follows existing pattern (`CHARON_PLUGINS_DIR`, `CHARON_CROWDSEC_*`) +- Operators can generate signatures with: `sha256sum plugin.so | awk '{print "sha256:" $1}'` + +### Deliverables + +1. **Parse and wire allowlist in main.go** +2. **Helper function to parse signature env var** +3. **Unit tests for PluginLoaderService** (currently missing!) +4. **Operator documentation** + +### Implementation Tasks + +#### Task 3.1: Add Signature Parsing Helper + +**File**: [backend/cmd/api/main.go](backend/cmd/api/main.go) (or new file `backend/internal/config/plugin_config.go`) + +```go +// parsePluginSignatures parses the CHARON_PLUGIN_SIGNATURES env var. +// Returns nil if unset/empty (permissive mode). +// Returns empty map if set to "{}" (strict mode, no plugins). +// Returns populated map if valid JSON with entries. +func parsePluginSignatures() (map[string]string, error) { + raw := os.Getenv("CHARON_PLUGIN_SIGNATURES") + if raw == "" { + return nil, nil // Permissive mode + } + + var sigs map[string]string + if err := json.Unmarshal([]byte(raw), &sigs); err != nil { + return nil, fmt.Errorf("invalid CHARON_PLUGIN_SIGNATURES JSON: %w", err) + } + return sigs, nil +} +``` + +#### Task 3.2: Wire Parsing into main.go + +**File**: [backend/cmd/api/main.go](backend/cmd/api/main.go) + +**Change** (around line 163): +```go +// Before: +pluginLoader := services.NewPluginLoaderService(db, pluginDir, nil) + +// After: +pluginSignatures, err := parsePluginSignatures() +if err != nil { + log.Fatalf("parse plugin signatures: %v", err) +} +if pluginSignatures != nil { + logger.Log().Infof("Plugin signature allowlist enabled with %d entries", len(pluginSignatures)) +} else { + logger.Log().Info("Plugin signature allowlist not configured (permissive mode)") +} +pluginLoader := services.NewPluginLoaderService(db, pluginDir, pluginSignatures) +``` + +#### Task 3.3: Create PluginLoaderService Unit Tests + +**File**: [backend/internal/services/plugin_loader_test.go](backend/internal/services/plugin_loader_test.go) (NEW) + +**Test Scenarios**: + +| Test Name | Setup | Expected Result | +|-----------|-------|-----------------| +| `TestNewPluginLoaderService_NilAllowlist` | `allowedSignatures: nil` | Service created, `allowedSigs` is nil | +| `TestNewPluginLoaderService_EmptyAllowlist` | `allowedSignatures: map[string]string{}` | Service created, `allowedSigs` is empty map | +| `TestNewPluginLoaderService_PopulatedAllowlist` | `allowedSignatures: {"test.so": "sha256:abc"}` | Service created with entries | +| `TestLoadPlugin_AllowlistEmpty_SkipsVerification` | Empty allowlist, mock plugin | Plugin loads without signature check | +| `TestLoadPlugin_AllowlistSet_PluginNotListed` | Allowlist without plugin | Returns `ErrPluginNotInAllowlist` | +| `TestLoadPlugin_AllowlistSet_SignatureMismatch` | Allowlist with wrong hash | Returns `ErrSignatureMismatch` | +| `TestLoadPlugin_AllowlistSet_SignatureMatch` | Allowlist with correct hash | Plugin loads successfully | +| `TestVerifyDirectoryPermissions_Secure` | Dir mode `0755` | Returns nil | +| `TestVerifyDirectoryPermissions_WorldWritable` | Dir mode `0777` | Returns error | +| `TestComputeSignature_ValidFile` | Real file | Returns `sha256:...` string | +| `TestLoadAllPlugins_DirectoryNotExist` | Non-existent dir | Returns nil (graceful skip) | +| `TestLoadAllPlugins_DirectoryInsecure` | World-writable dir | Returns error | + +**Note**: Testing actual `.so` loading requires CGO and platform-specific binaries. Focus unit tests on: +- Constructor behavior +- `verifyDirectoryPermissions()` (create temp dirs) +- `computeSignature()` (create temp files) +- Allowlist logic flow (mock the actual `plugin.Open` call) + +#### Task 3.4: Create parsePluginSignatures Unit Tests + +**File**: [backend/cmd/api/main_test.go](backend/cmd/api/main_test.go) or integrate into plugin_loader_test.go + +| Test Name | Env Value | Expected Result | +|-----------|-----------|-----------------| +| `TestParsePluginSignatures_Unset` | (not set) | `nil, nil` | +| `TestParsePluginSignatures_Empty` | `""` | `nil, nil` | +| `TestParsePluginSignatures_EmptyObject` | `"{}"` | `map[string]string{}, nil` | +| `TestParsePluginSignatures_Valid` | `{"a.so":"sha256:x"}` | `map with entry, nil` | +| `TestParsePluginSignatures_InvalidJSON` | `"not json"` | `nil, error` | +| `TestParsePluginSignatures_MultipleEntries` | `{"a.so":"sha256:x","b.so":"sha256:y"}` | `map with 2 entries, nil` | + +### Tasks & Owners + +- **Backend_Dev** + - [x] Create `parsePluginSignatures()` helper function ✅ *Completed 2026-01-14* + - [x] Update [backend/cmd/api/main.go](backend/cmd/api/main.go) to wire parsed signatures ✅ *Completed 2026-01-14* + - [ ] Create [backend/internal/services/plugin_loader_test.go](backend/internal/services/plugin_loader_test.go) with comprehensive test coverage + - [x] Add logging for allowlist mode (enabled vs permissive) ✅ *Completed 2026-01-14* +- **DevOps** + - [x] Ensure the plugin directory is mounted read-only in production (`/app/plugins:ro`) ✅ *Completed 2026-01-14* + - [x] Validate container permissions align with `verifyDirectoryPermissions()` (mode `0755` or stricter) ✅ *Completed 2026-01-14* + - [x] Document how to generate plugin signatures: `sha256sum plugin.so | awk '{print "sha256:" $1}'` ✅ *See below* +- **QA_Security** + - [x] Threat model review focused on `.so` loading risks ✅ *QA-approved 2026-01-14* + - [x] Verify error messages don't leak sensitive path information ✅ *QA-approved 2026-01-14* + - [x] Test edge cases: symlinks, race conditions, permission changes ✅ *QA-approved 2026-01-14* +- **Docs_Writer** + - [x] Create/update plugin operator docs explaining: ✅ *Completed 2026-01-14* + - `CHARON_PLUGIN_SIGNATURES` format and behavior + - How to compute signatures + - Recommended deployment pattern (read-only mounts, strict allowlist) + - Security implications of permissive mode + - [x] Created [docs/features/plugin-security.md](docs/features/plugin-security.md) ✅ *Completed 2026-01-14* + +### Acceptance Criteria + +- [x] Plugins load successfully when signature matches allowlist ✅ *QA-approved* +- [x] Plugins are rejected with `ErrPluginNotInAllowlist` when not in allowlist ✅ *QA-approved* +- [x] Plugins are rejected with `ErrSignatureMismatch` when hash differs ✅ *QA-approved* +- [x] World-writable plugin directory is detected and prevents all plugin loading ✅ *QA-approved* +- [x] Empty/unset `CHARON_PLUGIN_SIGNATURES` maintains backward compatibility (permissive) ✅ *QA-approved* +- [x] Invalid JSON in `CHARON_PLUGIN_SIGNATURES` causes startup failure with clear error ✅ *QA-approved* +- [ ] 100% patch coverage for modified lines in `main.go` +- [ ] New `plugin_loader_test.go` achieves high coverage of testable code paths +- [x] Operator documentation created: [docs/features/plugin-security.md](docs/features/plugin-security.md) ✅ *Completed 2026-01-14* + +### Verification Gates + +- Run backend coverage task: `shell: Test: Backend with Coverage` +- Run security scans: + - `shell: Security: CodeQL Go Scan (CI-Aligned) [~60s]` + - `shell: Security: Go Vulnerability Check` +- Run pre-commit: `shell: Lint: Pre-commit (All Files)` + +### Risks & Mitigations + +| Risk | Mitigation | +|------|------------| +| Invalid JSON crashes startup | Explicit error handling with descriptive message | +| Plugin name mismatch (with/without `.so`) | Document exact format; code expects filename as key | +| Signature format confusion | Enforce `sha256:` prefix; reject malformed signatures | +| Race condition: plugin modified after signature check | Document atomic deployment pattern (copy then rename) | +| Operators forget to update signatures after plugin update | Log warning when signature verification is enabled | + +--- + +## Phase 4 — E2E Coverage + Regression Safety + +**Status**: ✅ **Implementation Complete** (2026-01-15) +- 55 tests created across 3 test files +- All tests passing (52 pass, 3 conditional skip) +- Test files: `dns-provider-types.spec.ts`, `dns-provider-crud.spec.ts`, `manual-dns-provider.spec.ts` +- Fixtures created: `tests/fixtures/dns-providers.ts` + +### Current Test Coverage Analysis + +**Existing Test Files**: +| File | Purpose | Coverage Status | +|------|---------|-----------------| +| `tests/example.spec.js` | Playwright example (external site) | Not relevant to Charon | +| `tests/manual-dns-provider.spec.ts` | Manual DNS provider E2E tests | Good foundation, many tests skipped | + +**Existing `manual-dns-provider.spec.ts` Coverage**: +- ✅ Provider Selection Flow (navigation tests) +- ✅ Manual Challenge UI Display (conditional tests) +- ✅ Copy to Clipboard functionality +- ✅ Verify Button Interactions +- ✅ Accessibility Checks (keyboard navigation, ARIA) +- ✅ Component Tests (mocked API responses) +- ✅ Error Handling tests + +**Gaps Identified**: +1. **Types Endpoint Not Tested**: No tests verify `/api/v1/dns-providers/types` returns all provider types (built-in + custom + plugins) +2. **Provider Creation Flows**: No E2E tests for creating providers of each type +3. **Provider List Rendering**: No tests verify the provider cards grid renders correctly +4. **Edit/Delete Provider Flows**: No coverage for provider management operations +5. **Form Field Validation**: No tests for required field validation errors +6. **Dynamic Field Rendering**: No tests verify fields render from server-provided definitions +7. **Plugin Provider Types**: No tests for external plugin types (e.g., `powerdns`) + +### Deliverables + +1. **New Test File**: `tests/dns-provider-types.spec.ts` — Types endpoint and selector rendering +2. **New Test File**: `tests/dns-provider-crud.spec.ts` — Provider creation, edit, delete flows +3. **Updated Test File**: `tests/manual-dns-provider.spec.ts` — Enable skipped tests, add missing coverage +4. **Operator Smoke Test Documentation**: `docs/testing/e2e-smoke-tests.md` + +### Test File Organization + +``` +tests/ +├── example.spec.js # (Keep as Playwright reference) +├── manual-dns-provider.spec.ts # (Existing - Manual DNS challenge flow) +├── dns-provider-types.spec.ts # (NEW - Provider types endpoint & selector) +├── dns-provider-crud.spec.ts # (NEW - CRUD operations & validation) +└── dns-provider-a11y.spec.ts # (NEW - Focused accessibility tests) +``` + +### Test Scenarios (Prioritized) + +#### Priority 1: Core Functionality (Must Pass Before Merge) + +**File: `dns-provider-types.spec.ts`** + +| Test Name | Description | API Verified | +|-----------|-------------|--------------| +| `GET /dns-providers/types returns all built-in providers` | Verify cloudflare, route53, digitalocean, etc. in response | `GET /api/v1/dns-providers/types` | +| `GET /dns-providers/types includes custom providers` | Verify manual, webhook, rfc2136, script in response | `GET /api/v1/dns-providers/types` | +| `Provider selector dropdown shows all types` | Verify dropdown options match API response | UI + API | +| `Provider selector groups by category` | Built-in vs custom categorization | UI | +| `Provider type selection updates form fields` | Changing type loads correct credential fields | UI | + +**File: `dns-provider-crud.spec.ts`** + +| Test Name | Description | API Verified | +|-----------|-------------|--------------| +| `Create Cloudflare provider with valid credentials` | Complete create flow for built-in type | `POST /api/v1/dns-providers` | +| `Create Manual provider successfully` | Complete create flow for custom type | `POST /api/v1/dns-providers` | +| `Form shows validation errors for missing required fields` | Submit without required fields shows errors | UI validation | +| `Test Connection button shows success/failure` | Pre-save credential validation | `POST /api/v1/dns-providers/test` | +| `Edit provider updates name and settings` | Modify existing provider | `PUT /api/v1/dns-providers/:id` | +| `Delete provider with confirmation` | Delete flow with modal | `DELETE /api/v1/dns-providers/:id` | +| `Provider list renders all providers as cards` | Grid layout verification | `GET /api/v1/dns-providers` | + +#### Priority 2: Regression Safety (Manual DNS Challenge) + +**File: `manual-dns-provider.spec.ts`** (Enable and Update) + +| Test Name | Status | Action Required | +|-----------|--------|-----------------| +| `should navigate to DNS Providers page` | ✅ Active | Keep | +| `should show Add Provider button on DNS Providers page` | ⏭️ Skipped | **Enable** - requires backend | +| `should display Manual option in provider selection` | ⏭️ Skipped | **Enable** - requires backend | +| `should display challenge panel with required elements` | ✅ Conditional | Add mock data fixture | +| `Copy to clipboard functionality` | ✅ Conditional | Add fixture | +| `Verify button interactions` | ✅ Conditional | Add fixture | +| `Accessibility checks` | ✅ Partial | Expand coverage | + +**New Tests for Manual Flow**: +| Test Name | Description | +|-----------|-------------| +| `Create manual provider and verify in list` | Full create → list → verify flow | +| `Manual provider shows "Pending Challenge" state` | Verify UI state when challenge is active | +| `Manual challenge countdown timer decrements` | Time remaining updates correctly | +| `Manual challenge verification completes flow` | Success path when DNS propagates | + +#### Priority 3: Accessibility Compliance + +**File: `dns-provider-a11y.spec.ts`** + +| Test Name | WCAG Criteria | +|-----------|---------------| +| `Provider form has properly associated labels` | 1.3.1 Info and Relationships | +| `Error messages are announced to screen readers` | 4.1.3 Status Messages | +| `Keyboard navigation through form fields` | 2.1.1 Keyboard | +| `Focus visible on all interactive elements` | 2.4.7 Focus Visible | +| `Password fields are not autocompleted` | Security best practice | +| `Dialog trap focus correctly` | 2.4.3 Focus Order | +| `Form submission button has loading state` | 4.1.2 Name, Role, Value | + +#### Priority 4: Plugin Provider Types (Optional - When Plugins Present) + +**File: `dns-provider-crud.spec.ts`** (Conditional Tests) + +| Test Name | Condition | +|-----------|-----------| +| `External plugin types appear in selector` | `CHARON_PLUGINS_DIR` has `.so` files | +| `Create provider for plugin type (e.g., powerdns)` | Plugin type available in API | +| `Plugin provider test connection works` | Plugin credentials valid | + +### Implementation Guidance + +#### Test Data Strategy + +```typescript +// tests/fixtures/dns-providers.ts +export const mockProviderTypes = { + built_in: ['cloudflare', 'route53', 'digitalocean', 'googleclouddns'], + custom: ['manual', 'webhook', 'rfc2136', 'script'], +} + +export const mockCloudflareProvider = { + name: 'Test Cloudflare', + provider_type: 'cloudflare', + credentials: { + api_token: 'test-token-12345', + }, +} + +export const mockManualProvider = { + name: 'Test Manual', + provider_type: 'manual', + credentials: {}, +} +``` + +#### API Mocking Pattern (From Existing Tests) + +```typescript +// Mock provider types endpoint +await page.route('**/api/v1/dns-providers/types', async (route) => { + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ + types: [ + { type: 'cloudflare', name: 'Cloudflare', fields: [...] }, + { type: 'manual', name: 'Manual DNS', fields: [] }, + ], + }), + }); +}); +``` + +#### Test Structure Pattern (Following Existing Conventions) + +```typescript +import { test, expect } from '@playwright/test'; + +const BASE_URL = process.env.PLAYWRIGHT_BASE_URL || 'http://localhost:3003'; + +test.describe('DNS Provider Types', () => { + test.beforeEach(async ({ page }) => { + await page.goto(BASE_URL); + }); + + test('should display all provider types in selector', async ({ page }) => { + await test.step('Navigate to DNS Providers', async () => { + await page.goto(`${BASE_URL}/dns-providers`); + }); + + await test.step('Open Add Provider dialog', async () => { + await page.getByRole('button', { name: /add provider/i }).click(); + }); + + await test.step('Verify provider type options', async () => { + const providerSelect = page.getByRole('combobox', { name: /provider type/i }); + await providerSelect.click(); + + // Verify built-in providers + await expect(page.getByRole('option', { name: /cloudflare/i })).toBeVisible(); + await expect(page.getByRole('option', { name: /route53/i })).toBeVisible(); + + // Verify custom providers + await expect(page.getByRole('option', { name: /manual/i })).toBeVisible(); + }); + }); +}); +``` + +### Tasks & Owners + +- **QA_Security** + - [ ] Create `tests/dns-provider-types.spec.ts` with Priority 1 type tests + - [ ] Create `tests/dns-provider-crud.spec.ts` with Priority 1 CRUD tests + - [ ] Enable skipped tests in `tests/manual-dns-provider.spec.ts` + - [ ] Create `tests/dns-provider-a11y.spec.ts` with Priority 3 accessibility tests + - [ ] Create `tests/fixtures/dns-providers.ts` with mock data + - [ ] Document smoke test procedures in `docs/testing/e2e-smoke-tests.md` +- **Frontend_Dev** + - [ ] Fix any UI issues uncovered by E2E (focus order, error announcements, labels) + - [ ] Ensure form field IDs are stable for test selectors + - [ ] Add `data-testid` attributes where role-based selectors are insufficient +- **Backend_Dev** + - [ ] Fix any API contract mismatches discovered by E2E + - [ ] Ensure `/api/v1/dns-providers/types` returns complete field definitions + - [ ] Verify error response format matches frontend expectations + +### Potential Issues to Watch + +Based on code analysis, these may cause test failures (fix code first, per user directive): + +| Potential Issue | Component | Symptom | +|-----------------|-----------|---------| +| Types endpoint hardcoded | `dns_provider_handler.go` | Manual/plugin types missing from selector | +| Missing field definitions | API response | Form renders without credential fields | +| Dialog not trapping focus | `DNSProviderForm.tsx` | Tab escapes dialog | +| Select not keyboard accessible | `ui/Select.tsx` | Cannot navigate with arrow keys | +| Toast not announced | `toast.ts` | Screen readers miss success/error messages | + +### Acceptance Criteria + +- [ ] All Priority 1 tests pass reliably in Chromium +- [ ] All Priority 2 (manual provider regression) tests pass +- [ ] No skipped tests in `manual-dns-provider.spec.ts` (except documented exclusions) +- [ ] Priority 3 accessibility tests pass (or issues documented for fix) +- [ ] Smoke test documentation complete and validated by QA + +### Verification Gates + +1. **Run Playwright E2E first**: `npx playwright test --project=chromium` +2. **If tests fail**: Analyze whether failure is test bug or application bug + - Application bug → Fix code first, then re-run tests + - Test bug → Fix test, document reasoning +3. **After E2E passes**: Run full verification suite + - Backend coverage: `shell: Test: Backend with Coverage` + - Frontend coverage: `shell: Test: Frontend with Coverage` + - TypeScript check: `shell: Lint: TypeScript Check` + - Pre-commit: `shell: Lint: Pre-commit (All Files)` + - Security scans: CodeQL + Trivy + Go Vulnerability Check + +--- + +## Phase 5 — Test Coverage Gaps (Required Before Merge) + +**Status**: ✅ **Complete** (2026-01-15) + +### Context + +DoD verification passed overall (85%+ coverage), but specific gaps were identified during Issue #21 / PR #461 completeness review. + +### Deliverables + +1. **Unit tests for `plugin_loader.go`** — ✅ Comprehensive tests already exist +2. **Cover missing line in `encryption_handler.go`** — ✅ Documented as defensive error handling, added tests +3. **Enable skipped E2E tests** — ✅ Validated full integration + +### Tasks & Owners + +#### Task 5.1: Create `plugin_loader_test.go` + +**File**: [backend/internal/services/plugin_loader_test.go](backend/internal/services/plugin_loader_test.go) + +**Status**: ✅ **Complete** — Tests already exist with comprehensive coverage + +- **Backend_Dev** + - [x] `TestNewPluginLoaderService_NilAllowlist` — ✅ Exists as `TestNewPluginLoaderServicePermissiveMode` + - [x] `TestNewPluginLoaderService_EmptyAllowlist` — ✅ Exists as `TestNewPluginLoaderServiceStrictModeEmpty` + - [x] `TestNewPluginLoaderService_PopulatedAllowlist` — ✅ Exists as `TestNewPluginLoaderServiceStrictModePopulated` + - [x] `TestVerifyDirectoryPermissions_Secure` — ✅ Exists as `TestVerifyDirectoryPermissions` (mode 0755) + - [x] `TestVerifyDirectoryPermissions_WorldWritable` — ✅ Exists as `TestVerifyDirectoryPermissions` (mode 0777) + - [x] `TestComputeSignature_ValidFile` — ✅ Exists as `TestComputeSignature` + - [x] `TestLoadAllPlugins_DirectoryNotExist` — ✅ Exists as `TestLoadAllPluginsNonExistentDirectory` + - [x] `TestLoadAllPlugins_DirectoryInsecure` — ✅ Exists as `TestLoadAllPluginsWorldWritableDirectory` + +**Additional tests found in existing file:** +- `TestComputeSignatureNonExistentFile` +- `TestComputeSignatureConsistency` +- `TestComputeSignatureLargeFile` +- `TestComputeSignatureSpecialCharactersInPath` +- `TestLoadPluginNotInAllowlist` +- `TestLoadPluginSignatureMismatch` +- `TestLoadPluginSignatureMatch` +- `TestLoadPluginPermissiveMode` +- `TestLoadAllPluginsEmptyDirectory` +- `TestLoadAllPluginsEmptyPluginDir` +- `TestLoadAllPluginsSkipsDirectories` +- `TestLoadAllPluginsSkipsNonSoFiles` +- `TestListLoadedPluginsEmpty` +- `TestIsPluginLoadedFalse` +- `TestUnloadNonExistentPlugin` +- `TestCleanupEmpty` +- `TestParsePluginSignaturesLogic` +- `TestSignatureWorkflowEndToEnd` +- `TestGenerateUUIDUniqueness` +- `TestGenerateUUIDFormat` +- `TestConcurrentPluginMapAccess` + +**Note**: Actual `.so` loading requires CGO and is platform-specific. Tests focus on testable paths: +- Constructor behavior +- `verifyDirectoryPermissions()` with temp directories +- `computeSignature()` with temp files +- Allowlist validation logic + +#### Task 5.2: Cover `encryption_handler.go` Missing Line + +**Status**: ✅ **Complete** — Added documentation tests, identified defensive error handling + +- **Backend_Dev** + - [x] Identify uncovered line (likely error path in decrypt/encrypt flow) + - **Finding**: Lines 162-179 (`Validate` error path) require `ValidateKeyConfiguration()` to fail + - **Root Cause**: This only fails if `rs.currentKey == nil` (impossible after successful service creation) + - **Conclusion**: This is defensive error handling; cannot be triggered without mocking + - [x] Add targeted test case to reach 100% patch coverage + - Added `TestEncryptionHandler_Rotate_AuditChannelFull` — Tests audit channel saturation scenario + - Added `TestEncryptionHandler_Validate_ValidationFailurePath` — Documents the untestable path + +**Tests Added** (in `encryption_handler_test.go`): +- `TestEncryptionHandler_Rotate_AuditChannelFull` — Covers audit logging edge case +- `TestEncryptionHandler_Validate_ValidationFailurePath` — Documents limitation + +**Coverage Analysis**: +- `Validate` function at 60% — The uncovered 40% is defensive error handling +- `Rotate` function at 92.9% — Audit start log failure (line 63) is also defensive +- These paths exist for robustness but cannot be triggered in production without internal state corruption + +#### Task 5.3: Enable Skipped E2E Tests + +**Status**: ✅ **Previously Complete** (Phase 4) + +- **QA_Security** + - [x] Review skipped tests in `tests/manual-dns-provider.spec.ts` + - [x] Enable tests that have backend support + - [x] Document any tests that remain skipped with rationale + +### Acceptance Criteria + +- [x] `plugin_loader_test.go` exists with comprehensive coverage ✅ +- [x] 100% patch coverage for modified lines in PR #461 ✅ (Defensive paths documented) +- [x] All E2E tests enabled (or documented exclusions) ✅ +- [x] All verification gates pass ✅ + +### Verification Gates (Completed) + +- [x] Backend coverage: All plugin loader and encryption handler tests pass +- [x] E2E tests: Previously completed in Phase 4 +- [x] Pre-commit: No new lint errors introduced + +--- + +## Phase 6 — User Documentation (Recommended) + +**Status**: ✅ **Complete** (2026-01-15) + +### Context + +Core functionality is complete. User-facing documentation has been updated to reflect the new DNS Challenge feature. + +### Completed +- ✅ Rewrote `docs/features.md` as marketing overview (249 lines, down from 1,952 — 87% reduction) +- ✅ Added DNS Challenge feature to features.md with provider list and key benefits +- ✅ Organized features into 8 logical categories with "Learn More" links +- ✅ Created comprehensive `docs/features/dns-challenge.md` (DNS Challenge documentation) +- ✅ Created 18 feature stub pages for documentation consistency +- ✅ Updated README.md to include DNS Challenge in Top Features + +### Deliverables + +1. ~~**Rewrite `docs/features.md`**~~ — ✅ Complete (marketing overview style per new guidelines) +2. ~~**DNS Challenge Feature Docs**~~ — ✅ Complete (`docs/features/dns-challenge.md`) +3. ~~**Feature Stub Pages**~~ — ✅ Complete (18 stubs created) +4. ~~**Update README**~~ — ✅ Complete (DNS Challenge added to Top Features) + +### Tasks & Owners + +#### Task 6.1: Create DNS Troubleshooting Guide + +**File**: [docs/features/dns-troubleshooting.md](docs/features/dns-troubleshooting.md) (NEW) + +- **Docs_Writer** + - [ ] Common issues section: + - DNS propagation delays (TTL) + - Incorrect API credentials + - Missing permissions (e.g., Zone:Edit for Cloudflare) + - Firewall blocking outbound DNS API calls + - [ ] Verification steps: + - How to check if TXT record exists: `dig TXT _acme-challenge.example.com` + - How to verify credentials work before certificate request + - [ ] Provider-specific gotchas: + - Cloudflare: Zone ID vs API Token scopes + - Route53: IAM policy requirements + - DigitalOcean: API token permissions + +#### Task 6.2: Create Provider Quick-Setup Guides + +**Files**: +- [docs/providers/cloudflare.md](docs/providers/cloudflare.md) (NEW) +- [docs/providers/route53.md](docs/providers/route53.md) (NEW) +- [docs/providers/digitalocean.md](docs/providers/digitalocean.md) (NEW) + +- **Docs_Writer** + - [ ] Step-by-step credential creation (with screenshots/links) + - [ ] Required permissions/scopes + - [ ] Example Charon configuration + - [ ] Testing the provider connection + +#### Task 6.3: Update README Feature List + +**File**: [README.md](README.md) + +- **Docs_Writer** + - [ ] Add DNS Challenge / Wildcard Certificates to feature list + - [ ] Link to detailed documentation + +### Acceptance Criteria + +- [ ] DNS troubleshooting guide covers top 5 common issues +- [ ] At least 3 provider quick-setup guides exist +- [ ] README mentions wildcard certificate support +- [ ] Documentation follows markdown lint rules + +### Verification Gates + +- Run markdown lint: `npm run lint:md` +- Manual review of documentation accuracy + +--- + +## Open Questions (Need Explicit Decisions) + +- ~~For plugin signature allowlisting: what is the desired configuration shape?~~ + - **DECIDED: Option A (minimal)**: env var `CHARON_PLUGIN_SIGNATURES` with JSON map `pluginFilename.so` → `sha256:...` parsed by [backend/cmd/api/main.go](backend/cmd/api/main.go). See Phase 3 for full specification. + - ~~**Option B (operator-friendly)**: load from a mounted file path (adds new config surface)~~ — Not chosen; JSON env var is sufficient and simpler. +- For “first-party” providers (`webhook`, `script`, `rfc2136`): are these still required given external plugins already exist? + +--- + +## Notes on Accessibility + +UI work in this plan is built with accessibility in mind, but likely still requires manual review and testing (e.g., with Accessibility Insights) as changes land. diff --git a/tests/dns-provider-types.spec.ts b/tests/dns-provider-types.spec.ts index 5cea9e3e..7c3e040a 100644 --- a/tests/dns-provider-types.spec.ts +++ b/tests/dns-provider-types.spec.ts @@ -258,9 +258,9 @@ test.describe('DNS Provider Types', () => { }); await test.step('Verify Script path/command field appears', async () => { - const scriptField = page - .getByLabel(/script|command|path/i) - .or(page.getByRole('textbox', { name: /script|command|path/i })); + // Script provider shows "Script Path" field with placeholder "/scripts/dns-challenge.sh" + const scriptField = page.getByRole('textbox', { name: /script path/i }) + .or(page.getByPlaceholder(/dns-challenge\.sh/i)); await expect(scriptField).toBeVisible(); }); });