Files
Gondulf/docs/designs/phase-4b-security-hardening.md
Phil Skentelbery d3c3e8dc6b feat(security): merge Phase 4b security hardening
Complete security hardening implementation including HTTPS enforcement,
security headers, rate limiting, and comprehensive security test suite.

Key features:
- HTTPS enforcement with HSTS support
- Security headers (CSP, X-Frame-Options, X-Content-Type-Options)
- Rate limiting for all critical endpoints
- Enhanced email template security
- 87% test coverage with security-specific tests

Architect approval: 9.5/10

Generated with Claude Code
Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-20 18:28:50 -07:00

1812 lines
62 KiB
Markdown
Raw Permalink Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Phase 4b: Security Hardening
**Architect**: Claude (Architect Agent)
**Date**: 2025-11-20
**Status**: Design Complete - Clarifications Provided
**Phase**: 4b (Security Hardening)
**Design References**:
- `/docs/reports/2025-11-20-gap-analysis-v1.0.0.md` - Gap analysis Components 4-5
- `/docs/designs/phase-4-5-critical-components.md` - Phase 4-5 overview
- `/docs/architecture/security.md` - Security architecture
- `/docs/roadmap/v1.0.0.md` - Phase 4 requirements (lines 189-218)
- `/docs/designs/phase-4b-clarifications.md` - Implementation clarifications (2025-11-20)
---
## Purpose
Implement production-grade security hardening to protect against common web vulnerabilities and ensure compliance with OAuth 2.0 and IndieAuth security best practices. This phase adds the final security layer required for v1.0.0 production readiness.
**Critical Requirements**:
1. Security headers middleware (HSTS, CSP, X-Frame-Options, etc.)
2. HTTPS enforcement in production mode
3. Comprehensive security test suite
4. PII logging audit and remediation
5. Input validation verification
**Success Outcome**: Application is production-ready with defense-in-depth security measures, passing all security tests, and compliant with OWASP Top 10 recommendations.
---
## Specification References
**W3C IndieAuth**:
- Security Considerations section (HTTPS requirement)
- Token security (hashing, constant-time comparison)
**OAuth 2.0 Security Best Practices** (RFC 8252):
- HTTPS enforcement for non-localhost
- State parameter CSRF protection
- Open redirect prevention
- Token theft mitigation
**OWASP Top 10**:
- A01:2021 Broken Access Control (token validation)
- A03:2021 Injection (SQL injection, XSS)
- A05:2021 Security Misconfiguration (headers, HTTPS)
- A07:2021 Identification and Authentication Failures (timing attacks)
**v1.0.0 Roadmap Requirements**:
- Phase 4, lines 198-203: HTTPS enforcement, security headers, constant-time comparison, input sanitization, PII logging audit
- Exit criteria, lines 212-218: All security tests passing, no PII in logs
---
## Design Overview
Phase 4b implements four major components:
1. **Security Headers Middleware**: FastAPI middleware that adds security-related HTTP headers to all responses
2. **HTTPS Enforcement**: Production-mode middleware that redirects HTTP → HTTPS and validates TLS usage
3. **Security Test Suite**: Comprehensive tests covering timing attacks, injection prevention, XSS, open redirects, CSRF
4. **PII Logging Audit**: Review all logging statements and ensure no personally identifiable information is logged
**Architecture Pattern**: Defense-in-depth with multiple layers:
- Layer 1: Network (HTTPS/TLS)
- Layer 2: HTTP headers (prevent common attacks)
- Layer 3: Input validation (already implemented via Pydantic)
- Layer 4: Token security (hashing, constant-time comparison)
- Layer 5: Logging (no sensitive data exposure)
**Implementation Approach**:
- Middleware-based for headers and HTTPS enforcement (FastAPI middleware pattern)
- Pytest-based security tests using standard pytest markers
- Logging audit via grep and manual code review
- Configuration-driven (production vs development mode)
---
## Component 4: Security Headers Middleware
### Purpose
Add HTTP security headers to all responses to protect against clickjacking, XSS, MIME sniffing, and other client-side attacks. Headers are the first line of defense for browser-based vulnerabilities.
### Security Headers to Implement
| Header | Value | Purpose | Standard |
|--------|-------|---------|----------|
| `X-Frame-Options` | `DENY` | Prevent clickjacking attacks | RFC 7034 |
| `X-Content-Type-Options` | `nosniff` | Prevent MIME type sniffing | WHATWG |
| `X-XSS-Protection` | `1; mode=block` | Enable XSS filter (legacy browsers) | Non-standard but widely supported |
| `Strict-Transport-Security` | `max-age=31536000; includeSubDomains` | Force HTTPS for 1 year (production only) | RFC 6797 |
| `Content-Security-Policy` | `default-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' https:; frame-ancestors 'none'` | Restrict resource loading | W3C CSP Level 3 |
| `Referrer-Policy` | `strict-origin-when-cross-origin` | Control referrer information leakage | W3C Referrer Policy |
| `Permissions-Policy` | `geolocation=(), microphone=(), camera=()` | Disable unnecessary browser features | W3C Permissions Policy |
**Rationale for Values**:
- **X-Frame-Options: DENY**: IndieAuth server should never be embedded in frames (phishing risk)
- **X-Content-Type-Options: nosniff**: Force browser to respect Content-Type header (prevent XSS via misinterpreted files)
- **X-XSS-Protection: 1; mode=block**: Enable legacy XSS filter for older browsers (modern browsers have CSP)
- **Strict-Transport-Security**: Enforce HTTPS for 1 year including subdomains (production only, not development)
- **Content-Security-Policy**:
- `default-src 'self'`: Only load resources from same origin
- `style-src 'self' 'unsafe-inline'`: Allow inline styles for simplicity (templates may use inline styles)
- `img-src 'self' https:`: Allow images from same origin + HTTPS (for client logos from h-app)
- `frame-ancestors 'none'`: Equivalent to X-Frame-Options DENY
- **Referrer-Policy: strict-origin-when-cross-origin**: Send origin on cross-origin requests (not full URL, privacy)
- **Permissions-Policy**: Disable geolocation, microphone, camera (not needed for auth server)
### Middleware Design
**File**: `/src/gondulf/middleware/security_headers.py`
**Implementation**:
```python
"""Security headers middleware for Gondulf IndieAuth server."""
import logging
from typing import Callable
from fastapi import Request, Response
from starlette.middleware.base import BaseHTTPMiddleware
from gondulf.config import Config
logger = logging.getLogger("gondulf.middleware.security_headers")
class SecurityHeadersMiddleware(BaseHTTPMiddleware):
"""
Add security-related HTTP headers to all responses.
Headers protect against clickjacking, XSS, MIME sniffing, and other
client-side attacks. HSTS is only added in production mode (non-DEBUG).
References:
- OWASP Secure Headers Project
- Mozilla Web Security Guidelines
"""
def __init__(self, app, debug: bool = False):
"""
Initialize security headers middleware.
Args:
app: FastAPI application
debug: If True, skip HSTS header (development mode)
"""
super().__init__(app)
self.debug = debug
async def dispatch(self, request: Request, call_next: Callable) -> Response:
"""
Process request and add security headers to response.
Args:
request: Incoming HTTP request
call_next: Next middleware/handler in chain
Returns:
Response with security headers added
"""
# Process request
response = await call_next(request)
# Add security headers
response.headers["X-Frame-Options"] = "DENY"
response.headers["X-Content-Type-Options"] = "nosniff"
response.headers["X-XSS-Protection"] = "1; mode=block"
response.headers["Referrer-Policy"] = "strict-origin-when-cross-origin"
# CSP: Allow self, inline styles (for templates), and HTTPS images (for h-app logos)
response.headers["Content-Security-Policy"] = (
"default-src 'self'; "
"style-src 'self' 'unsafe-inline'; "
"img-src 'self' https:; "
"frame-ancestors 'none'"
)
# Permissions Policy: Disable unnecessary browser features
response.headers["Permissions-Policy"] = (
"geolocation=(), microphone=(), camera=()"
)
# HSTS: Only in production (not development)
if not self.debug:
response.headers["Strict-Transport-Security"] = (
"max-age=31536000; includeSubDomains"
)
logger.debug("Added HSTS header (production mode)")
return response
```
### Configuration Integration
**Update**: `/src/gondulf/main.py`
Add middleware registration after app initialization:
```python
from gondulf.middleware.security_headers import SecurityHeadersMiddleware
# ... existing imports and setup ...
# Add security headers middleware
app.add_middleware(
SecurityHeadersMiddleware,
debug=Config.DEBUG
)
logger.info(f"Security headers middleware registered (debug={Config.DEBUG})")
```
**Middleware Order**: Security headers should be added early in the middleware chain, but after any error-handling middleware. FastAPI applies middleware in reverse order of registration, so register security headers early.
### Testing Requirements
**File**: `/tests/integration/test_security_headers.py`
```python
"""Integration tests for security headers middleware."""
import pytest
from fastapi.testclient import TestClient
from gondulf.main import app
@pytest.fixture
def client():
"""FastAPI test client."""
return TestClient(app)
class TestSecurityHeaders:
"""Test security headers middleware."""
def test_x_frame_options_header(self, client):
"""Test X-Frame-Options header is present."""
response = client.get("/health")
assert "X-Frame-Options" in response.headers
assert response.headers["X-Frame-Options"] == "DENY"
def test_x_content_type_options_header(self, client):
"""Test X-Content-Type-Options header is present."""
response = client.get("/health")
assert "X-Content-Type-Options" in response.headers
assert response.headers["X-Content-Type-Options"] == "nosniff"
def test_x_xss_protection_header(self, client):
"""Test X-XSS-Protection header is present."""
response = client.get("/health")
assert "X-XSS-Protection" in response.headers
assert response.headers["X-XSS-Protection"] == "1; mode=block"
def test_csp_header(self, client):
"""Test Content-Security-Policy header is present and configured correctly."""
response = client.get("/health")
assert "Content-Security-Policy" in response.headers
csp = response.headers["Content-Security-Policy"]
assert "default-src 'self'" in csp
assert "style-src 'self' 'unsafe-inline'" in csp
assert "img-src 'self' https:" in csp
assert "frame-ancestors 'none'" in csp
def test_referrer_policy_header(self, client):
"""Test Referrer-Policy header is present."""
response = client.get("/health")
assert "Referrer-Policy" in response.headers
assert response.headers["Referrer-Policy"] == "strict-origin-when-cross-origin"
def test_permissions_policy_header(self, client):
"""Test Permissions-Policy header is present."""
response = client.get("/health")
assert "Permissions-Policy" in response.headers
policy = response.headers["Permissions-Policy"]
assert "geolocation=()" in policy
assert "microphone=()" in policy
assert "camera=()" in policy
def test_hsts_header_not_in_debug_mode(self, client, monkeypatch):
"""Test HSTS header is NOT present in debug mode."""
# This test assumes DEBUG=True in test environment
# In production, DEBUG=False and HSTS should be present
response = client.get("/health")
# Check current mode from Config
from gondulf.config import Config
if Config.DEBUG:
# HSTS should NOT be present in debug mode
assert "Strict-Transport-Security" not in response.headers
else:
# HSTS should be present in production mode
assert "Strict-Transport-Security" in response.headers
assert "max-age=31536000" in response.headers["Strict-Transport-Security"]
assert "includeSubDomains" in response.headers["Strict-Transport-Security"]
def test_headers_on_all_endpoints(self, client):
"""Test security headers are present on all endpoints."""
endpoints = [
"/",
"/health",
"/.well-known/oauth-authorization-server",
]
for endpoint in endpoints:
response = client.get(endpoint)
# All endpoints should have security headers
assert "X-Frame-Options" in response.headers
assert "X-Content-Type-Options" in response.headers
assert "Content-Security-Policy" in response.headers
def test_headers_on_error_responses(self, client):
"""Test security headers are present even on error responses."""
# Request non-existent endpoint (404)
response = client.get("/nonexistent")
assert response.status_code == 404
# Security headers should still be present
assert "X-Frame-Options" in response.headers
assert "X-Content-Type-Options" in response.headers
```
**Test Coverage**:
- Each security header individually
- Headers present on all endpoints
- HSTS conditional on production mode
- Headers present on error responses
---
## Component 5: HTTPS Enforcement
### Purpose
Ensure all production traffic uses HTTPS (TLS) to prevent credential theft, token interception, and man-in-the-middle attacks. Allow HTTP only for localhost development.
### Design Approach
**Strategy**: Configuration-driven enforcement based on `Config.DEBUG` and `Config.BASE_URL`:
- **Production Mode** (`DEBUG=False`): Reject HTTP requests or redirect to HTTPS
- **Development Mode** (`DEBUG=True`): Allow HTTP for localhost only
- **Validation**: Check `request.url.scheme` and `request.url.hostname`
**Implementation**: Middleware that runs before request processing.
### Middleware Design
**File**: `/src/gondulf/middleware/https_enforcement.py`
```python
"""HTTPS enforcement middleware for Gondulf IndieAuth server."""
import logging
from typing import Callable
from fastapi import Request, Response
from starlette.middleware.base import BaseHTTPMiddleware
from starlette.responses import RedirectResponse
from gondulf.config import Config
logger = logging.getLogger("gondulf.middleware.https_enforcement")
def is_https_request(request: Request) -> bool:
"""
Check if request is HTTPS, considering reverse proxy headers.
Args:
request: Incoming HTTP request
Returns:
True if HTTPS, False otherwise
"""
# Direct HTTPS
if request.url.scheme == "https":
return True
# Behind proxy - check forwarded header
# Only trust this header in production with TRUST_PROXY=true
if Config.TRUST_PROXY:
forwarded_proto = request.headers.get("X-Forwarded-Proto", "").lower()
return forwarded_proto == "https"
return False
class HTTPSEnforcementMiddleware(BaseHTTPMiddleware):
"""
Enforce HTTPS in production mode.
In production (DEBUG=False), reject or redirect HTTP requests to HTTPS.
In development (DEBUG=True), allow HTTP for localhost only.
Supports reverse proxy deployments via X-Forwarded-Proto header when
Config.TRUST_PROXY is enabled.
References:
- OAuth 2.0 Security Best Practices: HTTPS required
- W3C IndieAuth: TLS required for production
- Clarifications: See /docs/designs/phase-4b-clarifications.md section 2
"""
def __init__(self, app, debug: bool = False, redirect: bool = True):
"""
Initialize HTTPS enforcement middleware.
Args:
app: FastAPI application
debug: If True, allow HTTP for localhost (development mode)
redirect: If True, redirect HTTP to HTTPS. If False, return 400.
"""
super().__init__(app)
self.debug = debug
self.redirect = redirect
async def dispatch(self, request: Request, call_next: Callable) -> Response:
"""
Process request and enforce HTTPS if in production mode.
Args:
request: Incoming HTTP request
call_next: Next middleware/handler in chain
Returns:
Response (redirect to HTTPS, error, or normal response)
"""
hostname = request.url.hostname or ""
# Debug mode: Allow HTTP for localhost only
if self.debug:
if not is_https_request(request) and hostname not in ["localhost", "127.0.0.1", "::1"]:
logger.warning(
f"HTTP request to non-localhost in debug mode: {hostname}"
)
# Allow but log warning (for development on local networks)
# Continue processing
return await call_next(request)
# Production mode: Enforce HTTPS
if not is_https_request(request):
logger.warning(
f"HTTP request blocked in production mode: "
f"{request.method} {request.url}"
)
if self.redirect:
# Redirect HTTP → HTTPS
https_url = request.url.replace(scheme="https")
logger.info(f"Redirecting to HTTPS: {https_url}")
return RedirectResponse(url=str(https_url), status_code=301)
else:
# Return 400 Bad Request (strict mode)
from fastapi.responses import JSONResponse
return JSONResponse(
status_code=400,
content={
"error": "invalid_request",
"error_description": "HTTPS is required"
}
)
# HTTPS or allowed HTTP: Continue processing
return await call_next(request)
```
### Configuration Options
**Update `/src/gondulf/config.py`** to add security configuration:
```python
class Config:
"""Application configuration."""
# Existing configuration...
# Security configuration (Phase 4b)
HTTPS_REDIRECT: bool = Field(
default=True,
description="Redirect HTTP requests to HTTPS in production"
)
TRUST_PROXY: bool = Field(
default=False,
description="Trust X-Forwarded-* headers from reverse proxy"
)
SECURE_COOKIES: bool = Field(
default=True,
description="Set secure flag on cookies (requires HTTPS)"
)
@validator("HTTPS_REDIRECT")
def validate_https_redirect(cls, v, values):
"""Disable HTTPS redirect in development."""
if values.get("ENV") == "development":
return False
return v
```
**Environment Variables**:
- `GONDULF_HTTPS_REDIRECT=true` - Enable HTTPS redirect (default: true)
- `GONDULF_TRUST_PROXY=true` - Trust X-Forwarded-Proto header (default: false)
- `GONDULF_SECURE_COOKIES=true` - Set secure flag on cookies (default: true)
**Clarification**: See `/docs/designs/phase-4b-clarifications.md` section 2 for reverse proxy support details.
**Integration**: Add to `main.py`:
```python
from gondulf.middleware.https_enforcement import HTTPSEnforcementMiddleware
# Add HTTPS enforcement middleware (before security headers)
app.add_middleware(
HTTPSEnforcementMiddleware,
debug=Config.DEBUG,
redirect=Config.HTTPS_REDIRECT
)
logger.info(f"HTTPS enforcement middleware registered (debug={Config.DEBUG})")
```
**Middleware Order**:
1. HTTPS enforcement (first, redirects before processing)
2. Security headers (second, adds headers to all responses)
3. Application routers (last)
### Testing Requirements
**File**: `/tests/integration/test_https_enforcement.py`
```python
"""Integration tests for HTTPS enforcement middleware."""
import pytest
from fastapi.testclient import TestClient
from gondulf.main import app
@pytest.fixture
def client():
"""FastAPI test client."""
return TestClient(app)
class TestHTTPSEnforcement:
"""Test HTTPS enforcement middleware."""
def test_https_allowed_in_production(self, client, monkeypatch):
"""Test HTTPS requests are allowed in production mode."""
# Simulate production mode
from gondulf.config import Config
monkeypatch.setattr(Config, "DEBUG", False)
# HTTPS request should succeed
# Note: TestClient uses http by default, so this test is illustrative
# In real production, requests come from a reverse proxy (nginx) with HTTPS
response = client.get("/health")
assert response.status_code == 200
def test_http_localhost_allowed_in_debug(self, client, monkeypatch):
"""Test HTTP to localhost is allowed in debug mode."""
from gondulf.config import Config
monkeypatch.setattr(Config, "DEBUG", True)
# HTTP to localhost should succeed in debug mode
response = client.get("http://localhost:8000/health")
assert response.status_code == 200
def test_https_always_allowed(self, client):
"""Test HTTPS requests are always allowed regardless of mode."""
# HTTPS should work in both debug and production
response = client.get("/health")
# TestClient doesn't enforce HTTPS, but middleware should allow it
assert response.status_code == 200
```
**Note**: Full HTTPS testing requires integration tests with a real server (uvicorn with TLS). The TestClient doesn't enforce scheme validation, so these tests are illustrative. Real-world testing should be done in Phase 5 deployment testing.
### Production Deployment Notes
**Reverse Proxy Configuration** (nginx example):
```nginx
server {
listen 80;
server_name auth.example.com;
# Redirect all HTTP to HTTPS
return 301 https://$server_name$request_uri;
}
server {
listen 443 ssl http2;
server_name auth.example.com;
ssl_certificate /etc/letsencrypt/live/auth.example.com/fullchain.pem;
ssl_certificate_key /etc/letsencrypt/live/auth.example.com/privkey.pem;
# Proxy to Gondulf
location / {
proxy_pass http://localhost:8000;
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
proxy_set_header X-Forwarded-Proto $scheme;
proxy_set_header Host $host;
}
}
```
**Important**: The reverse proxy handles TLS termination. Gondulf sees HTTP from nginx but with `X-Forwarded-Proto: https` header. FastAPI's middleware respects this header.
---
## Component 6: Security Test Suite
### Purpose
Comprehensive security tests to verify protection against common vulnerabilities: timing attacks, SQL injection, XSS, open redirects, CSRF. Tests demonstrate compliance with OWASP Top 10 and OAuth 2.0 security best practices.
### Test Organization
**Directory**: `/tests/security/`
**Test Files**:
- `test_timing_attacks.py` - Timing attack resistance (token comparison)
- `test_sql_injection.py` - SQL injection prevention
- `test_xss_prevention.py` - Cross-site scripting prevention
- `test_open_redirect.py` - Open redirect prevention
- `test_csrf_protection.py` - CSRF protection (state parameter)
- `test_input_validation.py` - Input validation edge cases
**Pytest Marker**: Add `@pytest.mark.security` to all security tests for easy execution:
```bash
# Run only security tests
pytest -m security
# Run security tests with coverage
pytest -m security --cov=src/gondulf --cov-report=html
```
**Marker Registration**: See `/docs/designs/phase-4b-clarifications.md` section 7. Register the `security` marker in `pytest.ini` or `pyproject.toml` to prevent warnings and enable targeted test runs.
### Test 1: Timing Attack Resistance
**File**: `/tests/security/test_timing_attacks.py`
**Purpose**: Verify that token comparison uses constant-time algorithms to prevent timing side-channel attacks.
```python
"""Security tests for timing attack resistance."""
import secrets
import time
from statistics import mean, stdev
import pytest
from gondulf.services.token_service import TokenService
@pytest.mark.security
class TestTimingAttackResistance:
"""Test timing attack resistance in token validation."""
def test_token_verification_constant_time(self, db_session):
"""
Test that token verification takes similar time for valid and invalid tokens.
Timing attacks exploit differences in processing time to guess secrets.
This test verifies that token verification uses constant-time comparison.
"""
token_service = TokenService(db_session)
# Generate valid token
me = "https://user.example.com"
client_id = "https://client.example.com"
token = token_service.generate_access_token(
me=me,
client_id=client_id,
scope=""
)
# Measure time for valid token (hits database, passes validation)
valid_times = []
for _ in range(100):
start = time.perf_counter()
result = token_service.verify_access_token(token)
end = time.perf_counter()
valid_times.append(end - start)
assert result is not None # Valid token
# Measure time for invalid token (misses database, fails validation)
invalid_token = secrets.token_urlsafe(32)
invalid_times = []
for _ in range(100):
start = time.perf_counter()
result = token_service.verify_access_token(invalid_token)
end = time.perf_counter()
invalid_times.append(end - start)
assert result is None # Invalid token
# Statistical analysis: times should be similar
valid_mean = mean(valid_times)
invalid_mean = mean(invalid_times)
valid_stdev = stdev(valid_times)
invalid_stdev = stdev(invalid_times)
# Difference in means should be small relative to standard deviations
# Allow 3x stdev difference (99.7% confidence interval)
max_allowed_diff = 3 * max(valid_stdev, invalid_stdev)
actual_diff = abs(valid_mean - invalid_mean)
assert actual_diff < max_allowed_diff, (
f"Timing difference suggests timing attack vulnerability: "
f"valid={valid_mean:.6f}s, invalid={invalid_mean:.6f}s, "
f"diff={actual_diff:.6f}s, max_allowed={max_allowed_diff:.6f}s"
)
def test_hash_comparison_uses_constant_time(self):
"""
Test that hash comparison uses secrets.compare_digest.
This is a code inspection test (verified by reading token_service.py).
"""
import inspect
from gondulf.services.token_service import TokenService
source = inspect.getsource(TokenService.verify_access_token)
# Verify that secrets.compare_digest is used (or equivalent)
# OR that comparison happens in SQL (which is also constant-time)
assert "compare_digest" in source or "SELECT" in source, (
"Token verification should use constant-time comparison or SQL lookup"
)
def test_authorization_code_verification_constant_time(self, code_store):
"""
Test that authorization code verification is constant-time.
Similar to token verification test.
"""
# Store valid code
code = secrets.token_urlsafe(32)
code_data = {
"client_id": "https://client.example.com",
"redirect_uri": "https://client.example.com/callback",
"me": "https://user.example.com",
"state": "test-state"
}
code_store.store_authorization_code(code, code_data)
# Measure valid code lookup
valid_times = []
for _ in range(100):
start = time.perf_counter()
result = code_store.get_authorization_code(code)
end = time.perf_counter()
valid_times.append(end - start)
# Measure invalid code lookup
invalid_code = secrets.token_urlsafe(32)
invalid_times = []
for _ in range(100):
start = time.perf_counter()
result = code_store.get_authorization_code(invalid_code)
end = time.perf_counter()
invalid_times.append(end - start)
# Times should be similar
valid_mean = mean(valid_times)
invalid_mean = mean(invalid_times)
# In-memory dict lookup is very fast, so allow tighter bounds
# But still should be constant-time
assert abs(valid_mean - invalid_mean) < 0.001, (
"Authorization code lookup timing difference detected"
)
```
**Acceptance Criteria**:
- Token verification timing difference < 3x standard deviation
- Hash comparison uses `secrets.compare_digest` or SQL lookup
- Authorization code lookup is constant-time
**Implementation Note**: See `/docs/designs/phase-4b-clarifications.md` section 4 for handling timing test flakiness in CI environments (relaxed thresholds, increased samples, skip markers).
### Test 2: SQL Injection Prevention
**File**: `/tests/security/test_sql_injection.py`
**Purpose**: Verify that all database queries use parameterized queries and are not vulnerable to SQL injection.
```python
"""Security tests for SQL injection prevention."""
import pytest
from sqlalchemy.exc import SQLAlchemyError
from gondulf.services.token_service import TokenService
from gondulf.services.domain_verification import DomainVerificationService
@pytest.mark.security
class TestSQLInjectionPrevention:
"""Test SQL injection prevention in database queries."""
def test_token_service_sql_injection_in_me(self, db_session):
"""Test token service prevents SQL injection in 'me' parameter."""
token_service = TokenService(db_session)
# Attempt SQL injection via 'me' parameter
malicious_me = "https://user.example.com'; DROP TABLE tokens; --"
client_id = "https://client.example.com"
# Should not raise exception, should treat as literal string
token = token_service.generate_access_token(
me=malicious_me,
client_id=client_id,
scope=""
)
assert token is not None
# Verify token was stored safely (not executed as SQL)
result = token_service.verify_access_token(token)
assert result is not None
assert result["me"] == malicious_me # Stored as literal string
def test_domain_service_sql_injection_in_domain(self, db_session):
"""Test domain service prevents SQL injection in domain parameter."""
domain_service = DomainVerificationService(db_session)
# Attempt SQL injection via domain parameter
malicious_domain = "example.com'; DROP TABLE domains; --"
# Should not raise exception
# Note: This will fail validation (not a valid domain), but should not execute SQL
try:
domain_service.verify_domain_ownership(malicious_domain)
except ValueError:
# Expected: invalid domain format
pass
# Verify tables still exist (not dropped by injection)
result = db_session.execute("SELECT COUNT(*) FROM tokens")
assert result.scalar() >= 0 # Table exists
def test_token_lookup_sql_injection(self, db_session):
"""Test token lookup prevents SQL injection in token parameter."""
token_service = TokenService(db_session)
# Attempt SQL injection via token parameter
malicious_token = "' OR '1'='1"
# Should return None (not found), not execute malicious SQL
result = token_service.verify_access_token(malicious_token)
assert result is None
def test_parameterized_queries_used(self):
"""
Test that all database queries use parameterized queries.
This is a code inspection test.
"""
import ast
import inspect
from pathlib import Path
# Check all service files
services_dir = Path("src/gondulf/services")
violations = []
for service_file in services_dir.glob("*.py"):
source = service_file.read_text()
# Check for dangerous patterns (string formatting in SQL)
if 'f"SELECT' in source or "f'SELECT" in source:
violations.append(f"{service_file}: f-string in SQL query")
if '.format(' in source and 'SELECT' in source:
violations.append(f"{service_file}: .format() in SQL query")
if 'f"INSERT' in source or "f'INSERT" in source:
violations.append(f"{service_file}: f-string in INSERT query")
assert not violations, (
"SQL injection vulnerabilities detected:\n" + "\n".join(violations)
)
```
**Acceptance Criteria**:
- Malicious SQL strings treated as literal values
- No SQL execution from injected parameters
- All queries use parameterized format (behavioral testing, not source inspection)
**Implementation Note**: See `/docs/designs/phase-4b-clarifications.md` section 5. Tests should verify actual behavior (injection attempts fail safely) rather than inspecting source code patterns.
### Test 3: XSS Prevention
**File**: `/tests/security/test_xss_prevention.py`
**Purpose**: Verify that user input is properly escaped in HTML templates to prevent cross-site scripting.
```python
"""Security tests for XSS prevention."""
import pytest
from fastapi.testclient import TestClient
from gondulf.main import app
@pytest.mark.security
class TestXSSPrevention:
"""Test XSS prevention in HTML templates."""
def test_client_name_xss_escaped(self, client):
"""Test that client name is HTML-escaped in consent screen."""
# Mock h-app parser to return malicious client name
malicious_name = '<script>alert("XSS")</script>'
# Request authorization with malicious client metadata
# (This requires mocking the h-app parser)
# For now, test that templates auto-escape by inspecting template
from jinja2 import Environment
env = Environment(autoescape=True)
template_source = '{{ client_name }}'
template = env.from_string(template_source)
rendered = template.render(client_name=malicious_name)
# Should be escaped
assert '<script>' not in rendered
assert '&lt;script&gt;' in rendered
def test_me_parameter_xss_escaped(self):
"""Test that 'me' parameter is HTML-escaped in UI."""
from jinja2 import Environment
env = Environment(autoescape=True)
malicious_me = '<img src=x onerror="alert(1)">'
template_source = '<p>{{ me }}</p>'
template = env.from_string(template_source)
rendered = template.render(me=malicious_me)
# Should be escaped
assert '<img' not in rendered
assert '&lt;img' in rendered
def test_client_url_xss_escaped(self):
"""Test that client URL is HTML-escaped in consent screen."""
from jinja2 import Environment
env = Environment(autoescape=True)
malicious_url = 'javascript:alert(1)'
template_source = '<a href="{{ client_url }}">{{ client_url }}</a>'
template = env.from_string(template_source)
rendered = template.render(client_url=malicious_url)
# Should be escaped (Jinja2 escapes href attributes)
# Note: This might still be vulnerable if not validated at input
assert 'javascript:' in rendered # Jinja2 doesn't prevent javascript: in href
# So we need validation layer too (handled by Pydantic HttpUrl)
def test_jinja2_autoescape_enabled(self):
"""Test that Jinja2 autoescaping is enabled in templates."""
from gondulf.main import app
# FastAPI uses Jinja2Templates which has autoescape=True by default
# Verify this is the case
from fastapi.templating import Jinja2Templates
import inspect
# Find Jinja2Templates instances in the codebase
# Should have autoescape=True
# This is a configuration check
# For FastAPI's Jinja2Templates, autoescape is True by default
# So this test just confirms the default is used
templates = Jinja2Templates(directory="templates")
assert templates.env.autoescape == True
```
**Acceptance Criteria**:
- All user input HTML-escaped in templates
- Jinja2 autoescape enabled
- No raw HTML rendering without explicit marking
### Test 4: Open Redirect Prevention
**File**: `/tests/security/test_open_redirect.py`
**Purpose**: Verify that redirect_uri validation prevents open redirect attacks.
```python
"""Security tests for open redirect prevention."""
import pytest
from fastapi.testclient import TestClient
from gondulf.main import app
from gondulf.services.validation import validate_redirect_uri
@pytest.mark.security
class TestOpenRedirectPrevention:
"""Test open redirect prevention in authorization flow."""
def test_redirect_uri_must_match_client_id_domain(self):
"""Test that redirect_uri domain must match client_id domain."""
client_id = "https://client.example.com"
# Valid: same domain
is_valid, _ = validate_redirect_uri(
"https://client.example.com/callback",
client_id
)
assert is_valid
# Invalid: different domain
is_valid, _ = validate_redirect_uri(
"https://attacker.com/steal",
client_id
)
assert not is_valid
def test_redirect_uri_subdomain_allowed(self):
"""Test that redirect_uri subdomain of client_id is allowed."""
client_id = "https://example.com"
# Valid: subdomain
is_valid, _ = validate_redirect_uri(
"https://app.example.com/callback",
client_id
)
assert is_valid
def test_redirect_uri_rejects_open_redirect(self):
"""Test that common open redirect patterns are rejected."""
client_id = "https://client.example.com"
# Test various open redirect patterns
malicious_uris = [
"https://client.example.com@attacker.com/callback",
"https://client.example.com.attacker.com/callback",
"https://client.example.com/../../../attacker.com",
"https://attacker.com?client.example.com",
"https://attacker.com#client.example.com",
]
for uri in malicious_uris:
is_valid, _ = validate_redirect_uri(uri, client_id)
assert not is_valid, f"Open redirect allowed: {uri}"
def test_authorization_endpoint_validates_redirect_uri(self, client):
"""Test that authorization endpoint validates redirect_uri."""
# Attempt authorization with malicious redirect_uri
response = client.get(
"/authorize",
params={
"me": "https://user.example.com",
"client_id": "https://client.example.com",
"redirect_uri": "https://attacker.com/steal",
"state": "test-state",
"response_type": "code"
}
)
# Should return error (not redirect)
assert response.status_code == 400
assert "redirect_uri" in response.json().get("error_description", "")
def test_redirect_uri_must_be_https(self):
"""Test that redirect_uri must use HTTPS (except localhost)."""
client_id = "https://client.example.com"
# Invalid: HTTP for non-localhost
is_valid, _ = validate_redirect_uri(
"http://client.example.com/callback",
client_id
)
assert not is_valid
# Valid: HTTPS
is_valid, _ = validate_redirect_uri(
"https://client.example.com/callback",
client_id
)
assert is_valid
# Valid: HTTP for localhost (development)
is_valid, _ = validate_redirect_uri(
"http://localhost:3000/callback",
"http://localhost:3000"
)
assert is_valid
```
**Acceptance Criteria**:
- redirect_uri domain must match client_id domain or be subdomain
- Common open redirect patterns rejected
- HTTPS enforced (except localhost)
- Authorization endpoint validates redirect_uri
### Test 5: CSRF Protection
**File**: `/tests/security/test_csrf_protection.py`
**Purpose**: Verify that state parameter provides CSRF protection in authorization flow.
```python
"""Security tests for CSRF protection."""
import pytest
from fastapi.testclient import TestClient
from gondulf.main import app
@pytest.mark.security
class TestCSRFProtection:
"""Test CSRF protection via state parameter."""
def test_state_parameter_required(self, client):
"""Test that state parameter is required in authorization request."""
response = client.get(
"/authorize",
params={
"me": "https://user.example.com",
"client_id": "https://client.example.com",
"redirect_uri": "https://client.example.com/callback",
"response_type": "code"
# Missing state parameter
}
)
# Should return error
assert response.status_code == 400
assert "state" in response.json().get("error_description", "").lower()
def test_state_parameter_preserved_in_redirect(self, client, db_session):
"""Test that state parameter is included in redirect callback."""
# Complete authorization flow
# 1. Request authorization
state = "random-csrf-token-12345"
response = client.get(
"/authorize",
params={
"me": "https://user.example.com",
"client_id": "https://client.example.com",
"redirect_uri": "https://client.example.com/callback",
"state": state,
"response_type": "code"
}
)
# 2. Simulate user approval (POST to /authorize)
# ... (requires full integration test with domain verification)
# 3. Check that redirect includes state parameter
# In callback: https://client.example.com/callback?code=...&state=...
# This is tested in integration tests
def test_state_parameter_minimum_length(self, client):
"""Test that state parameter has minimum length for security."""
# Very short state (weak CSRF token)
response = client.get(
"/authorize",
params={
"me": "https://user.example.com",
"client_id": "https://client.example.com",
"redirect_uri": "https://client.example.com/callback",
"state": "123", # Too short
"response_type": "code"
}
)
# Should accept (OAuth 2.0 doesn't mandate minimum length)
# But recommend 32+ chars in documentation
# For v1.0.0, accept any non-empty state
def test_state_parameter_returned_unchanged(self, db_session):
"""Test that state parameter is returned unchanged (not modified)."""
from gondulf.services.authorization_service import AuthorizationService
auth_service = AuthorizationService(db_session)
original_state = "my-csrf-token-with-special-chars-!@#$%"
# Store authorization code
code = auth_service.generate_authorization_code(
me="https://user.example.com",
client_id="https://client.example.com",
redirect_uri="https://client.example.com/callback",
state=original_state
)
# Retrieve code data
code_data = auth_service.get_authorization_code(code)
# State should be unchanged
assert code_data["state"] == original_state
```
**Acceptance Criteria**:
- State parameter required in authorization requests
- State parameter preserved and returned in callback
- State parameter returned unchanged (no modification)
### Test 6: Input Validation Edge Cases
**File**: `/tests/security/test_input_validation.py`
**Purpose**: Verify that input validation handles edge cases and malicious input safely.
```python
"""Security tests for input validation."""
import pytest
from gondulf.services.validation import (
validate_url,
validate_redirect_uri,
validate_email_format
)
@pytest.mark.security
class TestInputValidation:
"""Test input validation edge cases and security."""
def test_url_validation_rejects_javascript_protocol(self):
"""Test that javascript: URLs are rejected."""
is_valid, _ = validate_url("javascript:alert(1)")
assert not is_valid
def test_url_validation_rejects_data_protocol(self):
"""Test that data: URLs are rejected."""
is_valid, _ = validate_url("data:text/html,<script>alert(1)</script>")
assert not is_valid
def test_url_validation_rejects_file_protocol(self):
"""Test that file: URLs are rejected."""
is_valid, _ = validate_url("file:///etc/passwd")
assert not is_valid
def test_url_validation_handles_unicode(self):
"""Test that URL validation handles Unicode safely."""
# IDN homograph attack
is_valid, _ = validate_url("https://аpple.com") # Cyrillic 'а'
# Should either reject or normalize to punycode
# For v1.0.0, basic validation (may accept)
def test_url_validation_handles_very_long_urls(self):
"""Test that URL validation handles very long URLs."""
long_url = "https://example.com/" + "a" * 10000
# Should handle without crashing (may reject)
try:
is_valid, _ = validate_url(long_url)
except Exception as e:
pytest.fail(f"URL validation crashed on long URL: {e}")
def test_email_validation_rejects_injection(self):
"""Test that email validation rejects injection attempts."""
malicious_emails = [
"user@example.com\nBcc: attacker@evil.com",
"user@example.com\r\nSubject: Injected",
"user@example.com<script>alert(1)</script>",
]
for email in malicious_emails:
is_valid = validate_email_format(email)
assert not is_valid, f"Email injection allowed: {email}"
def test_null_byte_injection_rejected(self):
"""Test that null byte injection is rejected."""
malicious_url = "https://example.com\x00.attacker.com"
is_valid, _ = validate_url(malicious_url)
assert not is_valid
```
**Acceptance Criteria**:
- Dangerous URL protocols rejected (javascript:, data:, file:)
- Email injection attempts rejected
- Null byte injection rejected
- Very long inputs handled safely
---
## Component 7: PII Logging Audit
### Purpose
Review all logging statements throughout the codebase to ensure no personally identifiable information (PII) is logged. PII includes email addresses, tokens, codes, and IP addresses.
### Audit Process
**Step 1: Grep for Logging Statements**
```bash
# Find all logging statements
grep -rn "logger\." src/gondulf/ > /tmp/logging_audit.txt
# Find potential PII in logs
grep -rn "logger.*email" src/gondulf/
grep -rn "logger.*token" src/gondulf/
grep -rn "logger.*code" src/gondulf/
grep -rn "logger.*password" src/gondulf/
```
**Step 2: Manual Review**
Review each logging statement for:
- Email addresses (PII)
- Full tokens or codes (security)
- Passwords (security)
- IP addresses (PII in some jurisdictions)
**Step 3: Remediation Patterns**
**SAFE Logging Patterns**:
```python
# GOOD: Domain only (public information)
logger.info(f"Authorization granted for {domain} to {client_id}")
# GOOD: Token prefix for correlation (first 8 chars)
logger.debug(f"Token generated: {token[:8]}...")
# GOOD: Error without sensitive data
logger.error(f"Email send failed for domain {domain}")
# GOOD: Count/aggregate data
logger.info(f"Cleaned up {count} expired tokens")
```
**UNSAFE Logging Patterns** (to fix):
```python
# BAD: Full email address
logger.info(f"Verification sent to {email}") # ← REMOVE email
# BAD: Full token
logger.debug(f"Token: {token}") # ← Use token[:8] only
# BAD: Full authorization code
logger.info(f"Code generated: {code}") # ← Use code[:8] only
# BAD: IP address
logger.info(f"Request from {request.client.host}") # ← Remove IP
```
**Step 4: Code Changes**
Create a checklist of files to update:
```markdown
## PII Logging Audit Checklist
- [ ] `/src/gondulf/services/email.py` - Remove email addresses from logs
- [ ] `/src/gondulf/services/token_service.py` - Use token[:8] prefix only
- [ ] `/src/gondulf/routers/authorization.py` - No PII in logs
- [ ] `/src/gondulf/routers/token.py` - No PII in logs
- [ ] `/src/gondulf/routers/verification.py` - Remove email from logs
- [ ] `/src/gondulf/services/domain_verification.py` - Remove email from logs
- [ ] `/src/gondulf/main.py` - Review startup logs
```
### Testing Requirements
**File**: `/tests/security/test_pii_logging.py`
```python
"""Security tests for PII in logging."""
import logging
import re
import pytest
@pytest.mark.security
class TestPIILogging:
"""Test that no PII is logged."""
def test_no_email_addresses_in_logs(self, caplog):
"""Test that email addresses are not logged."""
# Email regex pattern
email_pattern = r'\b[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Z|a-z]{2,}\b'
caplog.set_level(logging.DEBUG)
# Trigger various operations that might log email
# ... (requires integration test)
# Check logs for email addresses
for record in caplog.records:
match = re.search(email_pattern, record.message)
assert match is None, (
f"Email address found in log: {record.message}"
)
def test_no_full_tokens_in_logs(self, caplog):
"""Test that full tokens are not logged (only prefixes)."""
caplog.set_level(logging.DEBUG)
# Trigger token generation
# ... (requires integration test)
# Check logs for long alphanumeric strings (potential tokens)
for record in caplog.records:
# Tokens are 32+ chars of base64url
long_tokens = re.findall(r'[A-Za-z0-9_-]{32,}', record.message)
# If found, should be prefixed with "..." or truncated
for token in long_tokens:
assert "..." in record.message, (
f"Full token logged: {record.message}"
)
def test_no_passwords_in_logs(self, caplog):
"""Test that passwords are never logged."""
caplog.set_level(logging.DEBUG)
# Check all logs for "password" keyword
for record in caplog.records:
if "password" in record.message.lower():
# Should only be in config messages like "SMTP password: ***"
assert "***" in record.message or "password" in record.levelname.lower()
def test_logging_guidelines_documented(self):
"""Test that logging guidelines are documented."""
# Check for LOGGING.md or similar documentation
from pathlib import Path
docs_dir = Path("docs/standards")
coding_doc = docs_dir / "coding.md"
assert coding_doc.exists(), "Coding standards documentation missing"
content = coding_doc.read_text()
assert "logging" in content.lower()
assert "PII" in content or "personal" in content.lower()
```
**Acceptance Criteria**:
- No email addresses in logs (grep verification)
- No full tokens in logs (only 8-char prefixes with ellipsis suffix)
- No passwords in logs
- No IP addresses in logs (except rate limiting, temporary)
- Logging guidelines documented in `/docs/standards/coding.md`
**Implementation Notes**:
- See `/docs/designs/phase-4b-clarifications.md` section 3 for consistent token prefix format (exactly 8 chars + ellipsis)
- See section 8 for secure logging guidelines structure and examples to add to coding standards
---
## Error Handling
### Security Header Errors
**Scenario**: Middleware fails to add headers
**Handling**: Log error but continue processing (fail open, not fail closed). Security headers are defense-in-depth, not primary security.
```python
try:
response.headers["X-Frame-Options"] = "DENY"
except Exception as e:
logger.error(f"Failed to add security header: {e}")
# Continue anyway
```
### HTTPS Enforcement Errors
**Scenario**: Cannot determine request scheme
**Handling**: Assume HTTPS in production (conservative). Log warning.
```python
scheme = request.url.scheme or "https" # Default to HTTPS
if not scheme:
logger.warning("Cannot determine request scheme, assuming HTTPS")
```
### Security Test Failures
**Scenario**: Security test fails
**Handling**: Block merge/deployment. Security tests are blocking gates.
```yaml
# CI/CD
- name: Run Security Tests
run: pytest -m security
# If fails, pipeline stops (no merge)
```
---
## Security Considerations
### Defense-in-Depth
This phase implements multiple layers of security:
1. **Network**: HTTPS enforcement
2. **Headers**: Browser-level protections
3. **Input**: Validation (Pydantic, already implemented)
4. **Processing**: Constant-time operations
5. **Output**: HTML escaping
6. **Storage**: Hashed tokens, parameterized SQL
7. **Logging**: No PII exposure
### Threat Coverage
| Threat | Mitigation | Component |
|--------|------------|-----------|
| Clickjacking | X-Frame-Options: DENY | Security Headers |
| XSS | CSP + HTML escaping | Security Headers + Templates |
| MIME Sniffing | X-Content-Type-Options | Security Headers |
| Man-in-the-Middle | HTTPS + HSTS | HTTPS Enforcement + Headers |
| SQL Injection | Parameterized queries | Already implemented (verified by tests) |
| Timing Attacks | Constant-time comparison | Already implemented (verified by tests) |
| Open Redirects | redirect_uri validation | Already implemented (verified by tests) |
| CSRF | State parameter | Already implemented (verified by tests) |
| Privacy Violation | No PII logging | PII Audit |
### Residual Risks
**Accepted Risks** (v1.0.0):
- DDoS attacks (handled by infrastructure/CDN)
- Zero-day vulnerabilities in dependencies (monitoring required)
- Social engineering (user education)
- Compromised client applications (trust model limitation)
**Future Mitigations** (v1.1.0+):
- Rate limiting (prevent brute force)
- PKCE (additional token security)
- Token revocation (limit blast radius)
---
## Testing Strategy
### Test Organization
**Pytest Markers**:
```python
# Mark all security tests
@pytest.mark.security
# Run security tests only
pytest -m security
```
**Test Levels**:
- **Integration**: Security headers, HTTPS enforcement (middleware)
- **Security**: Timing attacks, injection, XSS, redirects, CSRF
- **Unit**: PII logging (caplog inspection)
**Coverage Target**: 100% of security-critical code
- Token validation: 100%
- Input validation: 100%
- Middleware: 100%
### Test Execution
**Local Development**:
```bash
# Run all security tests
pytest -m security -v
# Run with coverage
pytest -m security --cov=src/gondulf --cov-report=html
# Run specific security test file
pytest tests/security/test_timing_attacks.py -v
```
**CI/CD Pipeline**:
```yaml
security_tests:
stage: test
script:
- pytest -m security --cov=src/gondulf --cov-report=term --cov-report=xml
- bandit -r src/gondulf/ # Security linter
- pip-audit # Dependency vulnerability scan
artifacts:
reports:
coverage: coverage.xml
```
**Blocking Gate**: Security test failures block merge to main branch.
---
## Acceptance Criteria
### Component 4: Security Headers Middleware
- [ ] `SecurityHeadersMiddleware` class implemented in `/src/gondulf/middleware/security_headers.py`
- [ ] All 7 security headers added to responses:
- [ ] X-Frame-Options: DENY
- [ ] X-Content-Type-Options: nosniff
- [ ] X-XSS-Protection: 1; mode=block
- [ ] Strict-Transport-Security (production only)
- [ ] Content-Security-Policy (configured for templates + h-app logos)
- [ ] Referrer-Policy: strict-origin-when-cross-origin
- [ ] Permissions-Policy: geolocation=(), microphone=(), camera=()
- [ ] HSTS header conditional on production mode (not in DEBUG mode)
- [ ] Middleware registered in `main.py`
- [ ] All integration tests pass (`tests/integration/test_security_headers.py`)
- [ ] Headers present on all endpoints (tested)
- [ ] Headers present on error responses (tested)
### Component 5: HTTPS Enforcement
- [ ] `HTTPSEnforcementMiddleware` class implemented in `/src/gondulf/middleware/https_enforcement.py`
- [ ] HTTP requests blocked or redirected in production mode
- [ ] HTTP allowed for localhost in development mode
- [ ] Configuration option `GONDULF_HTTPS_REDIRECT` (default: true)
- [ ] Middleware registered in `main.py` (before security headers)
- [ ] Integration tests pass (`tests/integration/test_https_enforcement.py`)
- [ ] Production deployment documentation updated (nginx example)
### Component 6: Security Test Suite
- [ ] Security test directory created: `/tests/security/`
- [ ] All 6 security test files implemented:
- [ ] `test_timing_attacks.py` - Timing attack resistance tests pass
- [ ] `test_sql_injection.py` - SQL injection prevention tests pass
- [ ] `test_xss_prevention.py` - XSS prevention tests pass
- [ ] `test_open_redirect.py` - Open redirect prevention tests pass
- [ ] `test_csrf_protection.py` - CSRF protection tests pass
- [ ] `test_input_validation.py` - Input validation edge case tests pass
- [ ] All security tests marked with `@pytest.mark.security`
- [ ] All security tests pass: `pytest -m security`
- [ ] Security test coverage ≥95% for security-critical code
- [ ] CI/CD pipeline runs security tests (blocking gate)
### Component 7: PII Logging Audit
- [ ] All logging statements reviewed (grep audit completed)
- [ ] No email addresses in logs (verified by grep + tests)
- [ ] No full tokens in logs (only 8-char prefixes, verified)
- [ ] No passwords in logs (verified)
- [ ] No IP addresses in logs except rate limiting (verified)
- [ ] PII logging test implemented: `/tests/security/test_pii_logging.py`
- [ ] Logging guidelines documented in `/docs/standards/coding.md`
- [ ] Code changes committed with clear commit message
### Overall Phase 4b Completion
- [ ] All components implemented and tested
- [ ] All security tests passing
- [ ] No security warnings from bandit or pip-audit
- [ ] Code review completed (security focus)
- [ ] Documentation updated:
- [ ] `/docs/architecture/security.md` - Implementation confirmed
- [ ] `/docs/standards/coding.md` - Logging guidelines added
- [ ] Deployment guide includes HTTPS configuration
- [ ] Implementation report created: `/docs/reports/YYYY-MM-DD-phase-4b-security-hardening.md`
---
## Dependencies
**Upstream Dependencies** (must be complete before starting):
- Phase 1 (Foundation): COMPLETE ✅
- Phase 2 (Domain Verification): COMPLETE ✅
- Phase 3 (IndieAuth Protocol): COMPLETE ✅
- Phase 4a (Client Metadata): IN PROGRESS (parallel track)
**Downstream Dependencies** (blocked until complete):
- Phase 5 (Deployment & Testing): Requires Phase 4b complete
- v1.0.0 Release: Requires Phase 4b complete
**Parallel Work**:
- Phase 4a (Metadata Endpoint + Client Metadata): Can proceed in parallel
- Phase 4b (Security Hardening): Can proceed immediately
---
## Implementation Notes
### Estimated Effort
**Component 4: Security Headers Middleware**
- Implementation: 2-3 hours
- Testing: 2-3 hours
- **Total**: 0.5 days
**Component 5: HTTPS Enforcement**
- Implementation: 2-3 hours
- Testing: 1-2 hours
- **Total**: 0.5 days
**Component 6: Security Test Suite**
- Test 1 (Timing Attacks): 3-4 hours
- Test 2 (SQL Injection): 2-3 hours
- Test 3 (XSS Prevention): 2-3 hours
- Test 4 (Open Redirect): 2-3 hours
- Test 5 (CSRF Protection): 2-3 hours
- Test 6 (Input Validation): 2-3 hours
- **Total**: 2-2.5 days
**Component 7: PII Logging Audit**
- Grep audit: 1 hour
- Manual review: 2-3 hours
- Code changes: 2-3 hours
- Testing: 1-2 hours
- **Total**: 0.5-1 day
**Total Phase 4b Effort**: 3.5-4.5 days
### Recommended Implementation Order
1. **Day 1 Morning**: Security Headers Middleware (Component 4)
- Quick win, immediate security improvement
- Tests straightforward
2. **Day 1 Afternoon**: HTTPS Enforcement (Component 5)
- Pairs well with security headers
- Complete middleware layer
3. **Day 2**: PII Logging Audit (Component 7)
- Requires careful review
- Must be thorough
- Best done fresh
4. **Days 3-4**: Security Test Suite (Component 6)
- Most time-consuming
- Multiple test files
- Save for when middleware is complete
**Rationale**: Implement protections before verifying them. Security headers and HTTPS enforcement provide immediate value. PII audit is independent. Security tests verify everything at the end.
### Developer Notes
**Testing Considerations**:
- TestClient doesn't enforce HTTPS (limitation of test framework)
- Timing attack tests require statistical analysis (may be flaky)
- PII logging tests require caplog fixture and log level configuration
- Security headers tests are straightforward integration tests
**Code Review Focus**:
- Verify all security headers are correct
- Check HTTPS enforcement doesn't break development
- Ensure PII audit is complete (grep verification)
- Review timing attack test methodology
**Documentation**:
- Update deployment guide with HTTPS configuration (nginx)
- Add logging guidelines to coding standards
- Document security test execution in testing standards
---
## References
**Standards**:
- RFC 6797: HTTP Strict Transport Security (HSTS)
- RFC 7034: X-Frame-Options Header
- RFC 8414: OAuth 2.0 Authorization Server Metadata
- W3C Content Security Policy Level 3
- W3C IndieAuth Specification (Security Considerations)
- OWASP Top 10 (2021)
- OWASP Secure Headers Project
**Tools**:
- bandit: Python security linter
- pip-audit: Dependency vulnerability scanner
- pytest: Testing framework with security marker support
**Internal References**:
- `/docs/architecture/security.md` - Security architecture
- `/docs/standards/testing.md` - Testing standards
- `/docs/standards/coding.md` - Coding standards (to be updated)
- `/docs/reports/2025-11-20-gap-analysis-v1.0.0.md` - Gap analysis
---
**Design Status**: COMPLETE - CLARIFICATIONS PROVIDED
**Implementation Clarifications**: All Developer questions answered in `/docs/designs/phase-4b-clarifications.md` (2025-11-20):
1. CSP img-src allows any HTTPS source for client logos
2. HTTPS middleware checks X-Forwarded-Proto when TRUST_PROXY configured
3. Token logging uses consistent 8-char + ellipsis format
4. Timing tests use relaxed thresholds and increased samples in CI
5. SQL injection tests use behavioral testing, not source inspection
6. Security configuration (HTTPS_REDIRECT, TRUST_PROXY, SECURE_COOKIES) added to Config class
7. Pytest markers registered in pytest.ini/pyproject.toml
8. Comprehensive secure logging guidelines added to coding standards
**Next Steps**: Developer may proceed with implementation. Upon completion, Developer should create implementation report in `/docs/reports/YYYY-MM-DD-phase-4b-security-hardening.md`.
---
**Architect Sign-off**: This design is complete, all clarification questions are answered, and implementation may proceed. All security requirements from the v1.0.0 roadmap Phase 4 are addressed. Implementation of this design will satisfy exit criteria for Phase 4 (lines 212-218) and move v1.0.0 toward production readiness.