diff --git a/docs/plans/current_spec.md b/docs/plans/current_spec.md index 203e735c..5e8d763f 100644 --- a/docs/plans/current_spec.md +++ b/docs/plans/current_spec.md @@ -1,523 +1,60 @@ -# E2E Test Architecture Fix: Simulate Production Middleware Stack +# Reddit Feedback Implementation Plan: Logs UI, Caddy Import, Settings 400 Errors **Version:** 1.0 **Status:** Research Complete - Ready for Implementation -**Priority:** CRITICAL +**Priority:** HIGH **Created:** 2026-01-29 -**Author:** Planning Agent +**Source:** Reddit user feedback + +> **Note:** Previous active plan (E2E Test Architecture Fix) archived to [e2e_architecture_port80_spec.md](./e2e_architecture_port80_spec.md) + +--- + +## Active Plan + +See **[reddit_feedback_spec.md](./reddit_feedback_spec.md)** for the complete specification. + +--- + +## Quick Reference + +### Three Issues Addressed + +1. **Logs UI on widescreen** - Fixed `h-96` height, multi-span entries +2. **Caddy import not working** - Silent skipping, cryptic errors +3. **Settings 400 errors** - CIDR/URL validation, unfriendly messages + +### Key Files + +| Issue | Primary File | Line | +|-------|-------------|------| +| Logs UI | `frontend/src/components/LiveLogViewer.tsx` | 435 | +| Import | `backend/internal/api/handlers/import_handler.go` | 297 | +| Settings | `backend/internal/api/handlers/settings_handler.go` | 84 | + +### Implementation Timeline + +- **Day 1:** Quick wins (responsive height, error messages, normalization) +- **Day 2:** Core features (compact mode, skipped hosts, validation) +- **Day 3:** Polish (density control, import directive UI, inline validation) --- ## Executive Summary -**Problem:** E2E tests bypass Caddy middleware by hitting the Go backend directly (port 8080), creating a critical gap between test and production environments. Middleware (ACL, WAF, Rate Limiting, CrowdSec) never executes during E2E tests. +Three user-reported issues from Reddit: +1. **Logs UI** - Fixed height wastes screen space, entries wrap across multiple lines +2. **Caddy Import** - Silent failures, cryptic errors, missing feedback on skipped sites +3. **Settings 400** - Validation errors not user-friendly, missing auto-correction -**Root Cause [VERIFIED]:** Charon uses a **dual-serving architecture**: -- **Port 8080:** Backend serves frontend DIRECTLY via Gin (bypasses middleware) -- **Port 80:** Caddy serves frontend via `file_server` AND proxies API through middleware +**Root Causes Identified:** +- LiveLogViewer uses `h-96` fixed height, multi-span entries +- Import handler silently skips hosts without `reverse_proxy` +- Settings handler returns raw Go validation errors -**Solution:** Modify E2E test environment to route Playwright requests through Caddy (port 80) instead of directly to backend (port 8080), matching production architecture. - -**Verification Complete:** Code analysis confirms: -1. ✅ Caddy DOES serve frontend files via catch-all `file_server` handler -2. ✅ Caddy proxies API requests through full middleware stack -3. ✅ Port 80 tests the COMPLETE production flow (frontend + middleware + backend) -4. ✅ Port 8080 bypasses ALL middleware (development/fallback only) - -**Impact:** Enables true E2E testing of security middleware enforcement, removes all `test.skip()` statements, ensures production parity. +**Solution:** Responsive UI, enhanced error messages, input normalization --- -## 1. Architecture Analysis: Frontend Serving (VERIFIED) - -**CRITICAL FINDING:** Charon uses a **dual-serving architecture** where BOTH backend and Caddy serve the frontend. - -### Port 8080 (Backend Direct) - Development/Fallback - -``` -Browser → Backend:8080 → Gin Router - ├─ Frontend static files (via router.Static/StaticFile) - └─ API endpoints (/api/*) - -⚠️ NO MIDDLEWARE - Security features bypassed -``` - -**Source:** `backend/internal/server/server.go` lines 21-25 -```go -router.Static("/assets", frontendDir+"/assets") -router.StaticFile("/", frontendDir+"/index.html") -router.StaticFile("/banner.png", frontendDir+"/banner.png") -router.StaticFile("/logo.png", frontendDir+"/logo.png") -router.StaticFile("/favicon.png", frontendDir+"/favicon.png") -``` - -### Port 80 (Caddy Proxy) - Production Flow - -``` -Browser → Caddy:80 - ├─ Frontend UI (/*.html, /assets/*, images) - │ └─ Served by catch-all file_server handler - │ Source: backend/internal/caddy/config.go line 1136 - │ - └─ API Requests (/api/*) - └─ Caddy Middleware Pipeline: - ├─ CrowdSec Bouncer (IP blocking) - ├─ Coraza WAF (OWASP rules) - ├─ Rate Limiting (caddy-ratelimit) - └─ ACL (whitelist/blacklist) - └─ Reverse Proxy → Backend:8080 -``` - -**Source:** `backend/internal/caddy/config.go` lines 1136-1147 -```go -// Add catch-all 404 handler -// This matches any request that wasn't handled by previous routes -if frontendDir != "" { - catchAllRoute := &Route{ - Handle: []Handler{ - RewriteHandler("/unknown.html"), - FileServerHandler(frontendDir), // ← Serves frontend! - }, - Terminal: true, - } - routes = append(routes, catchAllRoute) -} -``` - -**Source:** `backend/internal/caddy/types.go` lines 230-235 -```go -func FileServerHandler(root string) Handler { - return Handler{ - "handler": "file_server", - "root": root, - } -} -``` - -### Why Port 80 is MANDATORY for E2E Tests - -| Aspect | Port 8080 | Port 80 | -|--------|-----------|---------| -| **Frontend Serving** | ✅ Gin static handlers | ✅ Caddy file_server | -| **API Requests** | ✅ Direct to backend | ✅ Through Caddy proxy | -| **CrowdSec** | ❌ Bypassed | ✅ Tested | -| **WAF (Coraza)** | ❌ Bypassed | ✅ Tested | -| **Rate Limiting** | ❌ Bypassed | ✅ Tested | -| **ACL** | ❌ Bypassed | ✅ Tested | -| **Production Flow** | ❌ Dev only | ✅ Real-world | - -**Decision:** Tests MUST run against port 80. Port 8080 bypasses the entire Caddy middleware stack, making E2E tests of Cerberus security features impossible. - ---- - -## 2. Problem Statement - -### Current E2E Flow (WRONG) -``` -Playwright Tests → Backend:8080 [BYPASSES CADDY & ALL MIDDLEWARE] -``` - -### Production Flow (CORRECT) -``` -User Request → Caddy:443/80 → [ACL, WAF, Rate Limit, CrowdSec] → Backend:8080 -``` - -### Requirements (EARS Notation) - -**R1 - Middleware Execution** -WHEN Playwright sends an HTTP request to the test environment, -THE SYSTEM SHALL route the request through Caddy on port 80. - -**R2 - Security Enforcement** -WHEN Caddy processes the request, -THE SYSTEM SHALL execute all configured middleware in the correct order. - -**R3 - Backend Isolation** -WHEN running E2E tests, -THE SYSTEM SHALL NOT allow direct access to backend port 8080 from Playwright. - ---- - -## 3. Root Cause Analysis - -### Current Docker Compose (`.docker/compose/docker-compose.playwright-local.yml`) - -```yaml -ports: - - "8080:8080" # ❌ Backend exposed directly - - "127.0.0.1:2019:2019" # Caddy admin API - - "2020:2020" # Emergency API - # ❌ MISSING: Port 80/443 for Caddy proxy -``` - -### Current Playwright Config (`playwright.config.js:90-110`) - -```javascript -use: { - baseURL: process.env.PLAYWRIGHT_BASE_URL || 'http://localhost:8080', - // ^^^^^^^^^^^^^ WRONG -} -``` - -### Container Architecture (Verified) - -**Services Running Inside `charon-e2e`:** - -1. **Caddy Proxy** (Confirmed in `docker-entrypoint.sh:274`) - - Listens: `0.0.0.0:80`, `0.0.0.0:443` - - Admin API: `0.0.0.0:2019` - - Middleware: ACL, WAF, Rate Limiting, CrowdSec - -2. **Go Backend** (Confirmed in `backend/cmd/api/main.go:275`) - - Listens: `0.0.0.0:8080` - - Provides: REST API, serves frontend - -**Key Findings:** -- ✅ Caddy IS running in E2E container -- ✅ Caddy listens on ports 80/443 internally -- ❌ Ports 80/443 NOT mapped in Docker Compose -- ❌ Tests hit port 8080 directly, bypassing Caddy - ---- - -## 4. Solution Design - -### Port Mapping Update - -**File:** `.docker/compose/docker-compose.playwright-local.yml` - -```yaml -ports: - - "80:80" # ✅ ADD: Caddy HTTP proxy - - "8080:8080" # KEEP: Management UI - - "127.0.0.1:2019:2019" # KEEP: Caddy admin API - - "2020:2020" # KEEP: Emergency API -``` - -### Playwright Config Update - -**File:** `playwright.config.js` - -```javascript -use: { - // OLD: baseURL: process.env.PLAYWRIGHT_BASE_URL || 'http://localhost:8080', - // NEW: Default to Caddy port - baseURL: process.env.PLAYWRIGHT_BASE_URL || 'http://localhost:80', -} -``` - -### Request Flow Post-Fix - -``` -Playwright Test - ↓ -http://localhost:80 (Caddy) - ↓ -Rate Limiter (if enabled) - ↓ -CrowdSec Bouncer (if enabled) - ↓ -Access Control Lists (if enabled) - ↓ -Coraza WAF (if enabled) - ↓ -Backend :8080 (proxied) - ↓ -Response -``` - ---- - -## 5. Implementation Plan - -### Phase 1: Docker Compose Update (5 min) - -**File:** `.docker/compose/docker-compose.playwright-local.yml` - -```yaml -# Add after line 13: -ports: - - "80:80" # ✅ ADD THIS LINE - - "8080:8080" - - "127.0.0.1:2019:2019" - - "2020:2020" -``` - -**Testing:** -```bash -.github/skills/scripts/skill-runner.sh docker-rebuild-e2e --clean -docker port charon-e2e | grep "80->" -# Expected: 0.0.0.0:80->80/tcp -curl -v http://localhost:80/api/v1/health -# Expected: HTTP/1.1 200 OK -``` - -### Phase 2: Playwright Config Update (2 min) - -**File:** `playwright.config.js` - -```javascript -// Line ~107: -use: { - baseURL: process.env.PLAYWRIGHT_BASE_URL || 'http://localhost:80', - // Change from :8080 to :80 ^^^ -} -``` - -### Phase 3: Environment Variable Setup (3 min) - -**File:** `.github/skills/test-e2e-playwright-scripts/run.sh` - -```bash -# Add after line ~30: -export PLAYWRIGHT_BASE_URL="${PLAYWRIGHT_BASE_URL:-http://localhost:80}" - -# Verify Caddy is accessible -if ! curl -sf "$PLAYWRIGHT_BASE_URL/api/v1/health" >/dev/null; then - log_error "Caddy proxy not responding at $PLAYWRIGHT_BASE_URL" - exit 1 -fi -``` - -### Phase 4: Health Check Enhancement (5 min) - -**File:** `.github/skills/docker-rebuild-e2e-scripts/run.sh` - -```bash -# Add in verify_environment() function: -log_info "Testing Caddy proxy path..." -if curl -sf http://localhost:80/api/v1/health &>/dev/null; then - log_success "Caddy proxy responding (port 80 → backend 8080)" -else - log_error "Caddy proxy not responding on port 80" - error_exit "Proxy path verification failed" -fi -``` - -### Phase 5: Remove test.skip() Statements (10 min) - -**Files:** `tests/security-enforcement/*.spec.ts` - -**Before:** -```typescript -test.skip('should block request from denied IP', async ({ page }) => { -``` - -**After:** -```typescript -test('should block request from denied IP', async ({ page }) => { -``` - -**Find all:** -```bash -grep -r "test.skip" tests/security-enforcement/ --include="*.spec.ts" -# Remove .skip from all security tests -``` - ---- - -## 6. Verification Strategy - -### Pre-Fix Baseline - -```bash -# Count skipped tests -grep -r "test.skip" tests/ --include="*.spec.ts" | wc -l - -# Check which port tests hit -tcpdump -i lo port 8080 or port 80 -c 10 & -npx playwright test tests/security-enforcement/acl-enforcement.spec.ts --project=chromium -# Expected: All traffic to port 8080 -``` - -### Post-Fix Validation - -```bash -# Rebuild -.github/skills/scripts/skill-runner.sh docker-rebuild-e2e --clean - -# Verify ports -docker port charon-e2e | grep "80->" - -# Test Caddy -curl -v http://localhost:80/api/v1/health - -# Run security tests -npx playwright test tests/security-enforcement/ --project=chromium - -# Check which port now -tcpdump -i lo port 8080 or port 80 -c 10 & -npx playwright test tests/security-enforcement/acl-enforcement.spec.ts --project=chromium -# Expected: All traffic to port 80 - -# Verify middleware executed -docker exec charon-e2e grep "rate_limit\|crowdsec\|waf\|acl" /var/log/caddy/access.log -``` - -### Middleware-Specific Tests - -**ACL:** -```bash -# Enable ACL, deny test IP -curl -X POST http://localhost:8080/api/v1/proxy-hosts/1/acl \ - -d '{"deny": ["127.0.0.1"]}' - -# Request through Caddy (should be blocked) -curl -v http://localhost:80/ -# Expected: HTTP/1.1 403 Forbidden -``` - -**WAF:** -```bash -# Enable WAF -curl -X POST http://localhost:8080/api/v1/security/waf -d '{"enabled": true}' - -# Send SQLi attack -curl -v http://localhost:80/?id=1%27%20OR%20%271%27=%271 -# Expected: HTTP/1.1 403 Forbidden -``` - -**Rate Limiting:** -```bash -# Enable rate limit -curl -X POST http://localhost:8080/api/v1/security/rate-limit -d '{"enabled": true, "limit": 10}' - -# Flood endpoint -for i in {1..15}; do curl http://localhost:80/ & done; wait - -# Check for 429 -curl -v http://localhost:80/ -# Expected: HTTP/1.1 429 Too Many Requests -``` - ---- - -## 7. Success Criteria - -| Metric | Current | Target | -|--------|---------|---------| -| Skipped security tests | ~15-20 | 0 | -| E2E test coverage | ~70% | 85%+ | -| Middleware test pass rate | 0% (skipped) | 100% | -| Port 80 traffic % | 0% | 100% | - -**Verification Script:** - -```bash -#!/bin/bash -# verify-e2e-architecture.sh - -# 1. Port mappings -if ! docker port charon-e2e | grep -q "80->80"; then - echo "❌ Port 80 not mapped"; exit 1 -fi - -# 2. Caddy accessibility -if ! curl -sf http://localhost:80/api/v1/health; then - echo "❌ Caddy not responding"; exit 1 -fi - -# 3. Security tests passing -if ! npx playwright test tests/security-enforcement/ --project=chromium 2>&1 | grep -q "passed"; then - echo "❌ Security tests not passing"; exit 1 -fi - -# 4. No skipped tests -if grep -r "test.skip" tests/security-enforcement/ --include="*.spec.ts"; then - echo "⚠️ WARNING: Tests still skipped" -fi - -echo "✅ E2E architecture correctly routes through Caddy" -``` - ---- - -## 8. Risk Assessment - -| Risk | Likelihood | Impact | Mitigation | -|------|-----------|--------|------------| -| Port 80 in use | Medium | High | Use alternate port (8081:80) | -| Breaking tests | Low | High | Run full suite before merge | -| Flaky tests | Medium | Medium | Add retry logic | - -**Port Conflict Resolution:** -```yaml -# Alternative: Use high port for Caddy -ports: - - "8081:80" # Caddy on alternate port -``` -```bash -export PLAYWRIGHT_BASE_URL="http://localhost:8081" -``` - ---- - -## 9. Rollout Plan - -**Week 1: Development Environment** -- Update compose file -- Test locally -- Validate middleware - -**Week 2: CI/CD Integration** -- Update workflows -- Test in CI -- Monitor stability - -**Week 3: Documentation** -- Update ARCHITECTURE.md -- Add troubleshooting guide -- Update testing.instructions.md - -**Week 4: Test Cleanup** -- Remove test.skip() -- Add new tests -- Verify 100% pass rate - ---- - -## Implementation Checklist - -- [ ] Phase 1: Update docker-compose.playwright-local.yml (add port 80:80) -- [ ] Phase 2: Update playwright.config.js (change baseURL to :80) -- [ ] Phase 3: Update test-e2e-playwright-scripts/run.sh (export PLAYWRIGHT_BASE_URL) -- [ ] Phase 4: Update docker-rebuild-e2e-scripts/run.sh (add proxy health check) -- [ ] Phase 5: Run full E2E test suite (verify all pass) -- [ ] Phase 6: Remove test.skip() from security enforcement tests -- [ ] Verification: Run verify-e2e-architecture.sh script -- [ ] Documentation: Update ARCHITECTURE.md -- [ ] Documentation: Update testing.instructions.md -- [ ] CI/CD: Update GitHub Actions workflows - ---- - -**Plan Status:** ✅ ARCHITECTURE VERIFIED - Port 80 is CORRECT and MANDATORY -**Confidence:** 100% - Full codebase analysis confirms Caddy serves frontend AND proxies API -**Next Step:** Backend_Dev to implement Phase 1-4 -**QA Step:** QA_Security to implement Phase 5-6 and verify - ---- - -## Related Files - -**Docker:** -- `.docker/compose/docker-compose.playwright-local.yml` -- `.docker/docker-entrypoint.sh` -- `Dockerfile` - -**Playwright:** -- `playwright.config.js` -- `tests/security-enforcement/*.spec.ts` - -**Skills:** -- `.github/skills/docker-rebuild-e2e-scripts/run.sh` -- `.github/skills/test-e2e-playwright-scripts/run.sh` - -**Backend:** -- `backend/internal/caddy/manager.go` -- `backend/internal/caddy/config.go` -- `backend/cmd/api/main.go` - ---- - -*End of Specification* +*For full specification, see [reddit_feedback_spec.md](./reddit_feedback_spec.md)* +*Previous E2E plan archived to [e2e_architecture_port80_spec.md](./e2e_architecture_port80_spec.md)* diff --git a/docs/plans/reddit_feedback_spec.md b/docs/plans/reddit_feedback_spec.md new file mode 100644 index 00000000..83750987 --- /dev/null +++ b/docs/plans/reddit_feedback_spec.md @@ -0,0 +1,1101 @@ +# Reddit Feedback Implementation Plan: Logs UI, Caddy Import, Settings 400 Errors + +**Version:** 1.1 +**Status:** Supervisor Review Complete - Ready for Implementation +**Priority:** HIGH +**Created:** 2026-01-29 +**Updated:** 2026-01-30 +**Source:** Reddit user feedback + +--- + +## Executive Summary + +Three user-reported issues from Reddit requiring investigation and fixes: + +1. **Logs UI on widescreen** - "logs on widescreen could be stretched and one line for better view (and more lines)" +2. **Caddy import not working** - "import from my caddy is not working" +3. **Settings 400 errors** - "i'm getting several i think error 400 on saving different settings" + +--- + +## Issue 1: Logs UI Widescreen Enhancement + +### Root Cause Analysis + +**Affected File:** [frontend/src/components/LiveLogViewer.tsx](../../frontend/src/components/LiveLogViewer.tsx) + +**Current Implementation (Lines 435-440):** +```tsx +
+``` + +**Problems Identified:** + +1. **Fixed height (`h-96` = 384px)** - Does not adapt to viewport, wastes vertical space on large monitors +2. **Multi-span log entries (Lines 448-497)** - Each log has multiple `` elements wrapped to new lines: + - Timestamp + - Source badge (security mode) + - Level badge (application mode) + - Client IP + - Message + - Block reason + - Status code + - Duration + - Details object +3. **No horizontal layout optimization** - Missing `whitespace-nowrap` for single-line display +4. **Font size fixed at `text-xs`** - No density options for users who prefer more/fewer lines + +### Requirements (EARS Notation) + +**R1.1 - Responsive Height** +WHEN the LiveLogViewer is rendered on a viewport taller than 768px, +THE SYSTEM SHALL expand the log container to use available vertical space (minimum 50vh). + +**R1.2 - Single-Line Log Format** +WHEN the user enables "compact mode", +THE SYSTEM SHALL display each log entry on a single line with horizontal scrolling. + +**R1.3 - Display Density Control** +WHERE the logs panel settings are available, +THE SYSTEM SHALL provide font size options: compact (text-xs), normal (text-sm), comfortable (text-base). + +**R1.4 - Preserve Existing Features** +WHEN displaying logs in any mode, +THE SYSTEM SHALL maintain all existing functionality: filtering, auto-scroll, pause, source badges, level colors. + +### Implementation Approach + +**Phase 1A: Responsive Height (Priority: Critical)** + +**File:** `frontend/src/components/LiveLogViewer.tsx` + +Replace fixed height with flex layout (avoids brittle pixel calculations): + +```tsx +// Wrap the log viewer in a flex container +
+
+ {/* Filter bar - fixed at top */} +
+
+ {/* Log entries */} +
+
+``` + +**Key Changes:** +- `flex flex-col` - Vertical flex container +- `flex-shrink-0` - Filter bar doesn't shrink +- `flex-1 min-h-0` - Log area grows to fill remaining space, `min-h-0` allows overflow scroll +- Removed brittle `calc(100vh-300px)` in favor of flex layout + +**Phase 1B: Single-Line Compact Mode (Priority: High)** + +Add state and UI toggle: + +```tsx +// Add after line ~55 (state declarations) +const [compactMode, setCompactMode] = useState(false); + +// Add toggle in filter bar (after line ~395) + +``` + +**Log Entry Scroll Behavior (UX Decision):** + +**Chosen: Option B - Truncate with Tooltip** + +Rationale: Per-entry horizontal scrolling (Option A) creates a poor UX with multiple scrollbars and makes it difficult to scan logs quickly. Truncation with tooltips provides: +- Clean visual appearance +- Full text available on hover +- No horizontal scrollbar clutter +- Better accessibility (screen readers announce full text) + +```tsx +{/* Log entry container - truncate long messages with tooltip */} +
+ {/* In compact mode, all spans are inline with flex-shrink-0 */} + {/* Timestamp */} + + {formatTimestamp(log.timestamp)} + + {/* Message - truncate in compact mode */} + + {log.message} + + {/* ... rest of spans */} +
+ +// Helper function for tooltip +const formatFullLogEntry = (log: LogEntry): string => { + return `${formatTimestamp(log.timestamp)} [${log.level}] ${log.client_ip || ''} ${log.message}`; +}; +``` + +**Phase 1C: Font Size/Density Control (Priority: Medium)** + +```tsx +// State for density +const [density, setDensity] = useState<'compact' | 'normal' | 'comfortable'>('compact'); + +// Density dropdown in filter bar + + +// Dynamic font class +const fontSizeClass = { + compact: 'text-xs', + normal: 'text-sm', + comfortable: 'text-base', +}[density]; + +// Apply to container +className={`... font-mono ${fontSizeClass} bg-black`} +``` + +### Testing Strategy + +**Unit Tests:** `frontend/src/components/__tests__/LiveLogViewer.test.tsx` +- Test compact mode toggle renders single-line entries +- Test density selection changes font class +- Test responsive height classes applied +- Test truncation with tooltip in compact mode + +**E2E Tests:** `tests/live-logs.spec.ts` +- Verify compact mode toggle works +- Verify tooltips show full log content on hover +- Verify density selector changes log appearance +- Visual regression tests for widescreen layout + +### Files to Modify + +| File | Changes | +|------|---------| +| [frontend/src/components/LiveLogViewer.tsx](../../frontend/src/components/LiveLogViewer.tsx#L435) | Responsive height, compact mode, density control | +| `tests/live-logs.spec.ts` | E2E tests for new features | + +### Complexity Estimate + +- Phase 1A (Responsive): **S** (1-2 hours) +- Phase 1B (Compact): **M** (3-4 hours) +- Phase 1C (Density): **S** (1-2 hours) + +--- + +## Issue 2: Caddy Import Not Working + +### Root Cause Analysis + +**Affected Files:** +- [backend/internal/api/handlers/import_handler.go](../../backend/internal/api/handlers/import_handler.go) +- [backend/internal/caddy/importer.go](../../backend/internal/caddy/importer.go) +- [frontend/src/pages/ImportCaddy.tsx](../../frontend/src/pages/ImportCaddy.tsx) + +**Import Flow:** +1. User uploads/pastes Caddyfile content +2. Backend writes to temp file (`import/uploads/.caddyfile`) +3. Calls `caddy adapt --config --adapter caddyfile` to convert to JSON +4. Parses JSON to extract `reverse_proxy` handlers +5. Returns hosts for review + +**Potential Failure Points Identified:** + +**Point A: Caddy Binary Not Available (Lines 12-17 of importer.go)** +```go +func (i *Importer) ValidateCaddyBinary() error { + _, err := i.executor.Execute(i.caddyBinaryPath, "version") + if err != nil { + return errors.New("caddy binary not found or not executable") + } + return nil +} +``` +- User's system may not have `caddy` in PATH +- Docker container has it, but local dev might not + +**Point B: Complex Caddyfile Syntax (Lines 280-286 of importer.go)** +```go +if handler.Handler == "rewrite" { + host.Warnings = append(host.Warnings, "Rewrite rules not supported") +} +if handler.Handler == "file_server" { + host.Warnings = append(host.Warnings, "File server directives not supported") +} +``` +- Only `reverse_proxy` handlers are extracted +- `file_server`, `rewrite`, `respond`, `redir` are not imported +- Snippet blocks and macros may confuse the parser + +**Point C: Import Directives Require Multi-File (Lines 297-305 of import_handler.go)** +```go +if len(result.Hosts) == 0 { + imports := detectImportDirectives(req.Content) + if len(imports) > 0 { + c.JSON(http.StatusBadRequest, gin.H{ + "error": "no sites found in uploaded Caddyfile; imports detected; please upload the referenced site files using the multi-file import flow", + "imports": imports, + }) + return + } +} +``` +- If Caddyfile uses `import ./sites.d/*`, hosts are in external files +- User must use multi-file upload flow but may not realize this + +**Point D: Silent Host Skipping (Lines 315-318 of importer.go)** +```go +if parsed.ForwardHost == "" || parsed.ForwardPort == 0 { + continue // Skip invalid entries +} +``` +- Hosts with no `reverse_proxy` backend are silently skipped +- No feedback to user about which sites were ignored + +**Point E: Parse Errors Not Surfaced** +- If `caddy adapt` fails, error is returned but may be cryptic +- User sees "import failed: " without actionable guidance + +### Requirements (EARS Notation) + +**R2.1 - Import Directive Detection** +WHEN user uploads a Caddyfile containing import directives, +THE SYSTEM SHALL detect the import paths and prompt user to use multi-file import flow. + +**R2.2 - Parse Error Clarity** +WHEN `caddy adapt` fails to parse the Caddyfile, +THE SYSTEM SHALL return a user-friendly error message with the line number and suggestion. + +**R2.3 - Skipped Hosts Feedback** +WHEN hosts are skipped due to missing `reverse_proxy` configuration, +THE SYSTEM SHALL include a list of skipped domains with reasons in the response. + +**R2.4 - Supported Directives Documentation** +WHEN displaying the import wizard, +THE SYSTEM SHALL show a list of supported Caddyfile directives and known limitations. + +**R2.5 - Caddy Binary Validation** +WHEN the import handler initializes, +THE SYSTEM SHALL validate that the Caddy binary is available and return a clear error if not. + +### Implementation Approach + +**Phase 2A: Enhanced Error Messages (Priority: Critical)** + +**File:** `backend/internal/caddy/importer.go` + +```go +// ParseCaddyfile - enhance error handling +func (i *Importer) ParseCaddyfile(caddyfilePath string) ([]byte, error) { + output, err := i.executor.Execute(i.caddyBinaryPath, "adapt", "--config", caddyfilePath, "--adapter", "caddyfile") + if err != nil { + // Parse caddy error output for line numbers + errMsg := string(output) + if strings.Contains(errMsg, "line") { + return nil, fmt.Errorf("Caddyfile syntax error: %s", extractLineError(errMsg)) + } + return nil, fmt.Errorf("failed to parse Caddyfile: %v", err) + } + return output, nil +} + +func extractLineError(errOutput string) string { + // Extract "line X: error message" format from caddy output + re := regexp.MustCompile(`(?i)line\s+(\d+):\s*(.+)`) + if match := re.FindStringSubmatch(errOutput); len(match) > 2 { + return fmt.Sprintf("Line %s: %s", match[1], match[2]) + } + return errOutput +} +``` + +**Phase 2B: Skipped Hosts Feedback (Priority: High)** + +**File:** `backend/internal/caddy/importer.go` + +```go +type ImportResult struct { + Hosts []ParsedHost + Skipped []SkippedHost // NEW: Add skipped hosts tracking + Warnings []string + ParsedAt time.Time +} + +type SkippedHost struct { + DomainNames string `json:"domain_names"` + Reason string `json:"reason"` +} + +// In ConvertToProxyHosts +func ConvertToProxyHostsWithSkipped(parsedHosts []ParsedHost) ([]models.ProxyHost, []SkippedHost) { + hosts := make([]models.ProxyHost, 0) + skipped := make([]SkippedHost, 0) + + for _, parsed := range parsedHosts { + if parsed.ForwardHost == "" || parsed.ForwardPort == 0 { + skipped = append(skipped, SkippedHost{ + DomainNames: parsed.DomainNames, + Reason: "No reverse_proxy backend defined", + }) + continue + } + // ... normal conversion + } + return hosts, skipped +} +``` + +**File:** `backend/internal/api/handlers/import_handler.go` + +```go +// In Upload handler response (line ~335) +c.JSON(http.StatusOK, gin.H{ + "session": gin.H{"id": sid, "state": "transient"}, + "preview": transient, + "conflict_details": conflictDetails, + "skipped_hosts": skippedHosts, // NEW: Include in response +}) +``` + +**Phase 2C: Frontend Skipped Hosts Display (Priority: High)** + +**File:** `frontend/src/pages/ImportCaddy.tsx` + +```tsx +// Add skipped hosts display in review step +// Note: Use snake_case to match backend JSON field naming convention +{importData.skipped_hosts?.length > 0 && ( +
+

+ Skipped Sites ({importData.skipped_hosts.length}) +

+

+ The following sites were not imported because they don't have a reverse proxy configuration: +

+
    + {importData.skipped_hosts.map((host: {domain_names: string, reason: string}) => ( +
  • {host.domain_names} - {host.reason}
    • + ))} +
+
+)} +``` + +**Phase 2D: Import Directive Auto-Detection UI (Priority: Medium)** + +**File:** `frontend/src/pages/ImportCaddy.tsx` + +When error contains `imports` array, show multi-file upload prompt: + +```tsx +// In error handling +if (error.imports && error.imports.length > 0) { + return ( +
+

+ Import Directives Detected +

+

+ Your Caddyfile references external files. Please use the multi-file import: +

+
    + {error.imports.map((imp: string) => ( +
  • {imp}
    • + ))} +
+ +
+ ); +} +``` + +### Testing Strategy + +**Unit Tests:** `backend/internal/caddy/importer_test.go` +- Test parse error extraction with line numbers +- Test skipped hosts tracking +- Test import directive detection + +**E2E Tests:** `tests/tasks/import-caddyfile.spec.ts` +- Test error message display for invalid Caddyfile +- Test skipped hosts warning display +- Test import directive detection and multi-file prompt + +**Backward Compatibility E2E Test:** `tests/tasks/import-caddyfile.spec.ts` +```typescript +test('existing import flow works for standard Caddyfile', async ({ page }) => { + // Navigate to import page + await page.goto('/import'); + + // Upload known-good Caddyfile with reverse_proxy + const caddyfileContent = ` + example.com { + reverse_proxy localhost:8080 + } + api.example.com { + reverse_proxy localhost:3000 + } + `; + + await page.getByTestId('caddyfile-input').fill(caddyfileContent); + await page.getByRole('button', { name: /parse|upload/i }).click(); + + // Verify hosts are detected + await expect(page.getByText('example.com')).toBeVisible(); + await expect(page.getByText('api.example.com')).toBeVisible(); + + // Verify no regression - import can proceed + await expect(page.getByRole('button', { name: /import|confirm/i })).toBeEnabled(); +}); +``` + +### Files to Modify + +| File | Changes | +|------|---------| +| [backend/internal/caddy/importer.go](../../backend/internal/caddy/importer.go) | Skipped host tracking, error parsing | +| [backend/internal/api/handlers/import_handler.go](../../backend/internal/api/handlers/import_handler.go#L335) | Include skipped hosts in response | +| [frontend/src/pages/ImportCaddy.tsx](../../frontend/src/pages/ImportCaddy.tsx) | Display skipped hosts, import directive UI | + +### Complexity Estimate + +- Phase 2A (Error Messages): **S** (1-2 hours) +- Phase 2B (Skipped Feedback Backend): **M** (2-3 hours) +- Phase 2C (Skipped Feedback UI): **S** (1-2 hours) +- Phase 2D (Import Directive UI): **M** (2-3 hours) + +--- + +## Issue 3: Error 400 on Saving Settings + +### Root Cause Analysis + +**Affected File:** [backend/internal/api/handlers/settings_handler.go](../../backend/internal/api/handlers/settings_handler.go) + +**400 Error Sources Identified:** + +| Line | Condition | Current Error Message | +|------|-----------|----------------------| +| 84-85 | `ShouldBindJSON` failure | `err.Error()` (cryptic Go validation) | +| 88-91 | Invalid admin whitelist CIDR | `"Invalid admin_whitelist: "` | +| 136-143 | `syncAdminWhitelist` failure | `"Invalid admin_whitelist"` | +| 185-192 | Bulk settings bind failure | `err.Error()` | +| 418-420 | SMTP config bind failure | `err.Error()` | +| 480-484 | Test email bind failure | `err.Error()` | + +**Common User Mistakes:** + +1. **Admin Whitelist CIDR Format** + - User enters: `192.168.1.1` (no CIDR suffix) + - Expected: `192.168.1.1/32` or `192.168.1.0/24` + - Error: `"Invalid admin_whitelist: invalid CIDR notation"` + +2. **Public URL Format** + - User enters: `mysite.com` (no scheme) + - Expected: `https://mysite.com` + - Error: `"Invalid URL format"` + +3. **Missing Required Fields** + - User sends: `{"key": "theme"}` (missing value) + - Error: `"Key: 'UpdateSettingRequest.Value' Error:Field validation for 'Value' failed on the 'required' tag"` + +4. **Boolean String Format** + - User sends: `{"value": true}` (JSON boolean instead of string) + - Error: JSON binding error + +### Requirements (EARS Notation) + +**R3.1 - User-Friendly Validation Errors** +WHEN a setting validation fails, +THE SYSTEM SHALL return a clear error message explaining the expected format with examples. + +**R3.2 - CIDR Auto-Correction** +WHEN user enters an IP without CIDR suffix for admin_whitelist, +THE SYSTEM SHALL automatically append `/32` for IPv4 single IPs and `/128` for IPv6 single IPs. + +**R3.3 - URL Auto-Correction** +WHEN user enters a URL without scheme for public_url that contains a recognized TLD, +THE SYSTEM SHALL automatically prepend `https://`. +WHEN user enters a private IP, localhost, or domain without TLD, +THE SYSTEM SHALL leave the value unchanged (prompting validation error if scheme is required). + +**R3.4 - Frontend Validation** +WHEN user enters a value in a settings form, +THE SYSTEM SHALL validate the format client-side before submission. + +**R3.5 - Specific Error Field Identification** +WHEN multiple settings are submitted via bulk update, +THE SYSTEM SHALL identify which specific setting failed validation. + +### Implementation Approach + +**Phase 3A: Backend Error Message Enhancement (Priority: Critical)** + +**File:** `backend/internal/api/handlers/settings_handler.go` + +```go +// Enhanced UpdateSetting error handling +func (h *SettingsHandler) UpdateSetting(c *gin.Context) { + var req UpdateSettingRequest + if err := c.ShouldBindJSON(&req); err != nil { + // Map validation errors to user-friendly messages + c.JSON(http.StatusBadRequest, gin.H{ + "error": formatValidationError(err), + "details": gin.H{"field": extractFieldName(err)}, + }) + return + } + + // Auto-correct common mistakes + if req.Key == "security.admin_whitelist" { + req.Value = normalizeCIDR(req.Value) + if err := validateAdminWhitelist(req.Value); err != nil { + c.JSON(http.StatusBadRequest, gin.H{ + "error": "Invalid IP/CIDR format. Use format like 192.168.1.0/24 or 10.0.0.1/32", + "details": gin.H{ + "field": "admin_whitelist", + "received": req.Value, + "examples": []string{"192.168.1.0/24", "10.0.0.0/8", "192.168.1.100/32"}, + }, + }) + return + } + } + + if req.Key == "general.public_url" { + req.Value = normalizeURL(req.Value) + } + // ... rest of handler +} + +// Auto-append /32 for IPv4 and /128 for IPv6 single IPs +func normalizeCIDR(value string) string { + entries := strings.Split(value, ",") + normalized := make([]string, 0, len(entries)) + for _, entry := range entries { + entry = strings.TrimSpace(entry) + if entry == "" { + continue + } + if !strings.Contains(entry, "/") { + // Check if it's a valid IP first + ip := net.ParseIP(entry) + if ip != nil { + if ip.To4() != nil { + entry = entry + "/32" // IPv4 + } else { + entry = entry + "/128" // IPv6 + } + } + } + normalized = append(normalized, entry) + } + return strings.Join(normalized, ", ") +} + +// Auto-prepend https:// for URLs with recognized TLDs only +// Private IPs and localhost are left unchanged for explicit user handling +func normalizeURL(value string) string { + value = strings.TrimSpace(value) + if value == "" { + return value + } + if strings.HasPrefix(value, "http://") || strings.HasPrefix(value, "https://") { + return value + } + + // Extract hostname (before any port or path) + host := value + if colonIdx := strings.Index(value, ":"); colonIdx != -1 { + host = value[:colonIdx] + } + if slashIdx := strings.Index(host, "/"); slashIdx != -1 { + host = host[:slashIdx] + } + + // Don't auto-add scheme for private IPs, localhost, or no TLD + if isPrivateOrLocal(host) { + return value // Leave as-is; validation will prompt user if scheme needed + } + + // Check for recognized TLD (public domain) + if hasRecognizedTLD(host) { + return "https://" + value + } + + return value // Leave as-is for ambiguous cases +} + +// Check if host is private IP, localhost, or local network +func isPrivateOrLocal(host string) bool { + // Check localhost variants + if host == "localhost" || strings.HasSuffix(host, ".local") { + return true + } + + // Check if it's an IP address + ip := net.ParseIP(host) + if ip != nil { + // Check private ranges (RFC 1918, RFC 4193) + privateNets := []string{ + "10.0.0.0/8", "172.16.0.0/12", "192.168.0.0/16", // IPv4 private + "127.0.0.0/8", // IPv4 loopback + "::1/128", "fc00::/7", "fe80::/10", // IPv6 loopback/private/link-local + } + for _, cidr := range privateNets { + _, network, _ := net.ParseCIDR(cidr) + if network.Contains(ip) { + return true + } + } + } + return false +} + +// Check if host has a recognized public TLD +func hasRecognizedTLD(host string) bool { + // Common TLDs - can extend as needed + tlds := []string{".com", ".org", ".net", ".io", ".dev", ".app", ".co", ".me", ".info", ".biz", ".edu", ".gov"} + hostLower := strings.ToLower(host) + for _, tld := range tlds { + if strings.HasSuffix(hostLower, tld) { + return true + } + } + // Country code TLDs (2 chars after dot) + if parts := strings.Split(hostLower, "."); len(parts) >= 2 { + lastPart := parts[len(parts)-1] + if len(lastPart) == 2 { // Likely a ccTLD + return true + } + } + return false +} + +// Map Go validation errors to user-friendly messages +func formatValidationError(err error) string { + errStr := err.Error() + + switch { + case strings.Contains(errStr, "'required' tag"): + return "This field is required" + case strings.Contains(errStr, "'url' tag"): + return "Invalid URL format. Use format like https://example.com" + case strings.Contains(errStr, "'email' tag"): + return "Invalid email format" + case strings.Contains(errStr, "cannot unmarshal"): + return "Invalid value type. Expected a text string." + default: + return "Invalid input format" + } +} +``` + +**Phase 3B: Frontend Form Validation (Priority: High)** + +**File:** `frontend/src/pages/Settings.tsx` (or relevant settings component) + +```tsx +// CIDR validation helper - supports IPv4 and IPv6 +const validateCIDR = (value: string): string | null => { + if (!value.trim()) return null; // Empty is ok + + const entries = value.split(',').map(e => e.trim()); + for (const entry of entries) { + // Check IPv4 CIDR format + const ipv4Regex = /^(\d{1,3}\.){3}\d{1,3}(\/\d{1,2})?$/; + // Check IPv6 format (simplified - accepts common formats) + const ipv6Regex = /^([0-9a-fA-F:]+)(\/\d{1,3})?$/; + + if (!ipv4Regex.test(entry) && !ipv6Regex.test(entry)) { + return `Invalid format: ${entry}. Use format like 192.168.1.0/24 or 2001:db8::/32`; + } + + // Additional validation for IPv4 octets + if (ipv4Regex.test(entry)) { + const ip = entry.split('/')[0]; + const octets = ip.split('.').map(Number); + if (octets.some(o => o > 255)) { + return `Invalid IP: ${entry}. Octets must be 0-255`; + } + } + } + return null; +}; + +// URL validation helper - tolerates private IPs/localhost +const validateURL = (value: string): string | null => { + if (!value.trim()) return null; + + // Check for obviously malformed URLs + if (value.startsWith('://') || value === 'http://' || value === 'https://') { + return 'Invalid URL format. Use format like https://example.com'; + } + + // For URLs with scheme, validate structure + if (value.startsWith('http://') || value.startsWith('https://')) { + try { + new URL(value); + return null; + } catch { + return 'Invalid URL format. Use format like https://example.com'; + } + } + + // For values without scheme, allow private IPs and localhost without validation + // Backend will handle auto-normalization for public domains + return null; +}; + +// In form component +const [errors, setErrors] = useState>({}); + +const validateAndSubmit = async () => { + const newErrors: Record = {}; + + // Validate each field + if (formData.admin_whitelist) { + const err = validateCIDR(formData.admin_whitelist); + if (err) newErrors.admin_whitelist = err; + } + + if (formData.public_url) { + const err = validateURL(formData.public_url); + if (err) newErrors.public_url = err; + } + + if (Object.keys(newErrors).length > 0) { + setErrors(newErrors); + return; + } + + // Submit + await saveSetting(formData); +}; +``` + +**Phase 3C: Inline Validation UI (Priority: Medium)** + +```tsx +// Field with validation +
+ + setAdminWhitelist(e.target.value)} + onBlur={() => setErrors({...errors, admin_whitelist: validateCIDR(adminWhitelist) || ''})} + className={errors.admin_whitelist ? 'border-red-500' : ''} + placeholder="192.168.1.0/24, ::1/128" + /> + {errors.admin_whitelist && ( +

{errors.admin_whitelist}

+ )} +

+ Examples: 192.168.1.0/24, 10.0.0.1/32, ::1/128, 2001:db8::/32 +

+
+ +
+ + setPublicUrl(e.target.value)} + onBlur={() => setErrors({...errors, public_url: validateURL(publicUrl) || ''})} + className={errors.public_url ? 'border-red-500' : ''} + placeholder="https://example.com" + /> + {errors.public_url && ( +

{errors.public_url}

+ )} +

+ For public domains, https:// will be added automatically. Private IPs (192.168.x.x, localhost) require explicit scheme. +

+
+``` + +### Testing Strategy + +**Unit Tests:** `backend/internal/api/handlers/settings_handler_test.go` +- Test CIDR normalization (`192.168.1.1` → `192.168.1.1/32`) +- Test IPv6 CIDR normalization: + - `::1` → `::1/128` + - `2001:db8::1` → `2001:db8::1/128` + - `fe80::1` → `fe80::1/128` +- Test mixed IPv4/IPv6 lists: `192.168.1.1, ::1` → `192.168.1.1/32, ::1/128` +- Test URL normalization: + - `example.com` → `https://example.com` + - `mysite.org/path` → `https://mysite.org/path` + - `192.168.1.1:8080` → `192.168.1.1:8080` (unchanged - private IP) + - `localhost:3000` → `localhost:3000` (unchanged - localhost) + - `10.0.0.1` → `10.0.0.1` (unchanged - private IP) + - `https://example.com` → `https://example.com` (unchanged - already has scheme) + - `http://internal.local` → `http://internal.local` (unchanged - already has scheme) +- Test user-friendly error messages +- Test bulk settings with one invalid entry + +**Frontend Validation Unit Tests:** `frontend/src/hooks/__tests__/useSettings.test.ts` +```typescript +import { validateCIDR, validateURL } from '../useSettings'; + +describe('validateCIDR', () => { + test('accepts valid IPv4 CIDR', () => { + expect(validateCIDR('192.168.1.0/24')).toBeNull(); + expect(validateCIDR('10.0.0.1/32')).toBeNull(); + }); + + test('accepts valid IPv6 CIDR', () => { + expect(validateCIDR('::1/128')).toBeNull(); + expect(validateCIDR('2001:db8::/32')).toBeNull(); + }); + + test('accepts IP without CIDR (will be auto-normalized)', () => { + expect(validateCIDR('192.168.1.1')).toBeNull(); + expect(validateCIDR('::1')).toBeNull(); + }); + + test('accepts comma-separated list', () => { + expect(validateCIDR('192.168.1.1, 10.0.0.0/8, ::1')).toBeNull(); + }); + + test('rejects invalid IP', () => { + expect(validateCIDR('999.999.999.999')).toContain('Invalid'); + expect(validateCIDR('not-an-ip')).toContain('Invalid'); + }); + + test('empty value is valid', () => { + expect(validateCIDR('')).toBeNull(); + }); +}); + +describe('validateURL', () => { + test('accepts full URL with scheme', () => { + expect(validateURL('https://example.com')).toBeNull(); + expect(validateURL('http://localhost:3000')).toBeNull(); + }); + + test('accepts domain without scheme (will be auto-normalized for public)', () => { + expect(validateURL('example.com')).toBeNull(); + expect(validateURL('mysite.org')).toBeNull(); + }); + + test('accepts private IP without scheme', () => { + expect(validateURL('192.168.1.1:8080')).toBeNull(); + expect(validateURL('localhost:3000')).toBeNull(); + }); + + test('rejects malformed URL', () => { + expect(validateURL('http://')).toContain('Invalid'); + expect(validateURL('://no-scheme')).toContain('Invalid'); + }); + + test('empty value is valid', () => { + expect(validateURL('')).toBeNull(); + }); +}); +``` + +**E2E Tests:** `tests/settings.spec.ts` +- Test inline validation displays error on blur +- Test form submission blocked with invalid data +- Test successful save after fixing validation errors +- Test IPv6 CIDR accepts and saves correctly + +### Files to Modify + +| File | Changes | +|------|---------| +| [backend/internal/api/handlers/settings_handler.go](../../backend/internal/api/handlers/settings_handler.go#L84) | Normalization, enhanced errors | +| `backend/internal/api/handlers/settings_handler_test.go` | Unit tests for CIDR/URL normalization | +| `frontend/src/pages/Settings.tsx` | Client-side validation | +| `frontend/src/hooks/useSettings.ts` | Validation helpers (validateCIDR, validateURL) | +| `frontend/src/hooks/__tests__/useSettings.test.ts` | Unit tests for validation helpers | + +### Complexity Estimate + +- Phase 3A (Backend Normalization): **M** (2-3 hours) +- Phase 3B (Frontend Validation): **M** (3-4 hours) +- Phase 3C (Inline UI): **S** (1-2 hours) + +--- + +## Implementation Phases + +### Phase 1: Quick Wins (Day 1) + +| Task | Issue | Priority | Estimate | +|------|-------|----------|----------| +| 1A: Responsive log height | Logs UI | Critical | 1-2h | +| 2A: Enhanced error messages | Import | Critical | 1-2h | +| 3A: Backend normalization | Settings 400 | Critical | 2-3h | + +### Phase 2: Core Features (Day 2) + +| Task | Issue | Priority | Estimate | +|------|-------|----------|----------| +| 1B: Compact log mode | Logs UI | High | 3-4h | +| 2B: Skipped hosts backend | Import | High | 2-3h | +| 3B: Frontend validation | Settings 400 | High | 3-4h | + +### Phase 3: Polish (Day 3) + +| Task | Issue | Priority | Estimate | +|------|-------|----------|----------| +| 1C: Density control | Logs UI | Medium | 1-2h | +| 2C: Skipped hosts UI | Import | High | 1-2h | +| 2D: Import directive UI | Import | Medium | 2-3h | +| 3C: Inline validation UI | Settings 400 | Medium | 1-2h | + +--- + +## Testing Requirements + +### Unit Test Coverage + +- `frontend/src/components/__tests__/LiveLogViewer.test.tsx` - Compact mode, density +- `frontend/src/hooks/__tests__/useSettings.test.ts` - CIDR/URL validation helpers +- `backend/internal/caddy/importer_test.go` - Skipped hosts, error parsing +- `backend/internal/api/handlers/settings_handler_test.go` - Normalization, errors (IPv4/IPv6) + +### E2E Test Coverage + +- `tests/live-logs.spec.ts` - Widescreen layout, compact mode +- `tests/tasks/import-caddyfile.spec.ts` - Error messages, skipped hosts, backward compatibility +- `tests/settings.spec.ts` - Validation, 400 error handling + +### Patch Coverage Requirement + +All modified lines must have 100% patch coverage per Codecov requirements. + +--- + +## Success Criteria + +### Issue 1: Logs UI +- [ ] Log container height adapts to viewport using flex layout +- [ ] Compact mode displays single-line log entries with truncation +- [ ] Tooltips show full log content on hover in compact mode +- [ ] Density control allows font size selection +- [ ] Compact mode toggle has `aria-label` for accessibility +- [ ] All existing log features continue to work + +### Issue 2: Caddy Import +- [ ] Parse errors include line numbers and suggestions +- [ ] Skipped hosts are listed with reasons (using snake_case JSON fields) +- [ ] Import directive detection prompts multi-file flow +- [ ] Import succeeds for standard Caddyfile with `reverse_proxy` +- [ ] Backward compatibility E2E test passes for existing import flows + +### Issue 3: Settings 400 +- [ ] IPv4 CIDR entries auto-normalized (IP → IP/32) +- [ ] IPv6 CIDR entries auto-normalized (IP → IP/128) +- [ ] Mixed IPv4/IPv6 lists normalized correctly +- [ ] URLs auto-normalized only for public domains with TLDs +- [ ] Private IPs and localhost left unchanged (explicit user handling) +- [ ] Error messages are user-friendly with examples +- [ ] Client-side validation prevents invalid submissions +- [ ] Frontend validation helpers have 100% unit test coverage + +--- + +## Optional Enhancements (Post-MVP) + +The following suggestions are non-blocking improvements to consider after initial implementation: + +### Issue 1: Logs UI +- **Persist User Preferences:** Store `compactMode` and `density` settings in localStorage + ```tsx + useEffect(() => { + const saved = localStorage.getItem('logViewerPrefs'); + if (saved) { + const prefs = JSON.parse(saved); + setCompactMode(prefs.compactMode ?? false); + setDensity(prefs.density ?? 'compact'); + } + }, []); + + useEffect(() => { + localStorage.setItem('logViewerPrefs', JSON.stringify({ compactMode, density })); + }, [compactMode, density]); + ``` +- **Log Virtualization:** For very large log volumes, consider `react-window` for virtual scrolling + +### Issue 2: Caddy Import +- **Common Fixes Section:** Add a collapsible "Common Issues" panel with parse error suggestions: + - "Missing closing brace" → Check line X + - "Unknown directive" → List of supported directives + +### Issue 3: Settings 400 +- No additional optional enhancements identified + +--- + +## Risk Assessment + +| Risk | Likelihood | Impact | Mitigation | +|------|-----------|--------|------------| +| Breaking existing log filters | Low | High | Extensive unit/E2E tests | +| CIDR auto-correction edge cases (IPv4) | Medium | Medium | Thorough regex and net.ParseIP testing | +| IPv6 CIDR edge cases (compressed notation) | Medium | Medium | Test with ::1, 2001:db8::, fe80:: variants | +| URL heuristic false positives (auto-adding https://) | Low | Low | Only apply to recognized TLDs; fallback to original | +| Import changes break existing flows | Low | High | Backward compatibility E2E test | +| JSON field naming mismatch (frontend/backend) | Medium | Medium | Explicit json tags + TypeScript types | + +--- + +## Dependencies + +- No external dependencies required +- All changes are internal to existing components +- No database schema changes needed + +--- + +## Related Documentation + +- [ARCHITECTURE.md](../../ARCHITECTURE.md) - System overview +- [docs/features.md](../features.md) - Feature documentation +- [E2E Test Architecture Plan](./current_spec.md) - Previous active plan + +--- + +*End of Specification*