Skip to content

feat: comprehensive security hardening (10 findings)#27

Closed
damacus wants to merge 18 commits intomainfrom
security-hardening
Closed

feat: comprehensive security hardening (10 findings)#27
damacus wants to merge 18 commits intomainfrom
security-hardening

Conversation

@damacus
Copy link
Owner

@damacus damacus commented Mar 4, 2026

Summary

Systematic security hardening across 10 findings from a full audit:

High severity

  • Fix CSRF bypass: enforce token on all mutating requests, not just HTMX (internal/middleware/csrf.go)
  • Add rate limiting to /login POST: 10 req/min per IP, returns 429 (cmd/server/main.go)

Medium severity

  • Change GET /logout to POST /logout with CSRF-protected form to prevent forced-logout attacks
  • Replace unsafe-inline in CSP with per-request cryptographic nonce; inject into all templates and partials
  • Add structured audit logging (JSON via log/slog) for all requests after auth; includes user, method, path, status, IP
  • Session key validation: panic on startup in production if IRON_SESSION_KEY is absent or not 32 bytes

Low / hardening

  • Enforce upload size limit (default 100 MB, configurable via MAX_UPLOAD_SIZE)
  • Vendor Tailwind CSS v4, HTMX, Alpine.js (CSP build), and Lucide icons into views/static/; removes all CDN dependencies from CSP
  • Log a warning when MINIO_ENDPOINT is not set (uses localhost:9000 fallback)
  • Gate OIDC routes behind OIDC_ENABLED=true; routes are not registered by default

Also integrates upstream refactors: utils.IsSecureRequest, registerPublicRoutes/registerProtectedRoutes helpers, resolveMinioEndpoint, and oidcEnabledFromEnv.

Test plan

  • All existing tests pass (go test ./...)
  • POST to /login without token returns 400
  • POST to /login with 11 rapid requests returns 429 on the 11th
  • GET /logout returns 404; POST /logout with CSRF token redirects to /login
  • CSP header contains nonce- value; no unsafe-inline
  • Audit log line appears in stdout on every authenticated request
  • Server refuses to start in production (APP_ENV=production) with missing/short session key
  • Upload larger than 100 MB returns 413
  • Static assets served from /static/ (no CDN requests)
  • GET /login/oauth returns 404 without OIDC_ENABLED=true

damacus added 18 commits March 3, 2026 15:00
Generate a cryptographically random 16-byte base64 nonce on every request,
store it in the Echo context as "csp-nonce", and embed it in the CSP
script-src and style-src directives. Add nonce="{{ .CSPNonce }}" to all
<script> and <style> tags in base.html, browser.html, and login.html.
Records method, path, status, remote_ip, and authenticated user access key
for every request; mutating methods logged at INFO, safe methods at DEBUG.
Resolves 3-way merge conflicts between our 10-finding security hardening
implementation and 8 upstream commits including:
- Use utils.IsSecureRequest from upstream refactor (removes local copy)
- Adopt registerPublicRoutes/registerProtectedRoutes/resolveMinioEndpoint structure
- Keep our additions: rate limiter, POST logout, upload limit, audit log, static serving
- Keep our CSRF fix (non-HTMX POSTs require token, not skip)
- Update user_journey_test to use POST /logout
- C1: Replace GET /logout links in browser.html and settings.html with POST forms
- C2: Pass initialized map to login renderer so CSP nonce is injected correctly
- I1: Vendor Inter font (woff2) to views/static/fonts/, remove Google Fonts CDN
- I3: Document intentional CookieHTTPOnly=false decision in csrf.go
- I4: Replace log.Printf with slog.Warn in main.go and auth_service.go
- I5: Assert status code field in audit log test
The 4-node MinIO cluster + app takes longer than 10s to fully start.
Poll /health every 5s (up to 150s) so E2E tests only run once the
app is confirmed ready, eliminating flaky login timeouts.
- Remove x-show from creds form in login.html (always-visible default tab,
  Alpine.js initialization timing caused login timeouts in fresh browser context)
- Add waitForLoadState('domcontentloaded') in bucket_stats beforeEach after goto /login
- Fix user_management logout: replace broken a[href="/logout"] with
  form[action="/logout"] button selector (logout is now a POST form)
@damacus damacus closed this Mar 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant