From 8aa648142145b3a495c4be06a16d981d3a8f6b07 Mon Sep 17 00:00:00 2001 From: xarmian Date: Sun, 5 Apr 2026 10:26:00 -0400 Subject: [PATCH] PHASE-12: Security Hardening for Pad Cloud (#67) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: enforce RBAC role checks on all mutation endpoints (TASK-150) Add requireMinRole helper and role enforcement to 30+ mutation handlers. Viewers are now blocked from all state-changing operations, editors can mutate items/docs/comments/views but not collections/webhooks/workspace settings, and only owners can perform administrative operations. Includes 11 integration tests with real auth covering viewer/editor/owner access across items, collections, documents, comments, agent roles, item links, and workspace operations. * fix: scope search results to user's workspaces (TASK-151) Search without a ?workspace= param previously returned results from all workspaces in the database. Now the handler resolves the authenticated user's workspace memberships and passes their IDs to the store query, ensuring results only include items from workspaces the user belongs to. Fresh installs (no users) retain unscoped search for backward compat. Includes integration test proving cross-workspace isolation. * fix: add webhook URL validation and SSRF protection (TASK-152) Webhook creation now validates URLs before accepting them: only HTTP(S) schemes allowed, embedded credentials rejected, private/reserved IPs blocked (loopback, RFC1918, link-local, cloud metadata 169.254.169.254), and hostnames are DNS-resolved to verify they don't point to private IPs. Defense-in-depth check also added to the dispatcher's deliver function so existing webhooks with unsafe URLs are blocked at delivery time. * feat: add CSRF protection with double-submit cookie pattern (TASK-153) Implements CSRF middleware that validates X-CSRF-Token header matches the pad_csrf cookie on all state-changing API requests. Bearer token auth, auth endpoints, and fresh installs are exempt. The frontend client reads the CSRF cookie and attaches the header on mutations. * feat: add per-endpoint rate limiting middleware (TASK-154) Adds IP-based rate limiting for auth endpoints (5/min login, 3/hr password reset, 5/hr registration) and user-based limits for API (100/min) and search (30/min). Uses golang.org/x/time/rate with automatic stale-entry cleanup. Adds chi RealIP middleware for correct client IP behind proxies. Returns 429 with Retry-After. * fix: sanitize error responses and remove PII from logs (TASK-155) Replace all writeError(500, err.Error()) calls with writeInternalError that logs the real error server-side and returns a generic message to clients. Remove email addresses, user IDs, and password reset tokens from log output to prevent PII leakage. * feat: add security headers, configurable CORS, and secure cookies (TASK-160) Add SecurityHeaders middleware (CSP, X-Frame-Options, nosniff, Referrer-Policy, Permissions-Policy). Make CORS origins configurable via PAD_CORS_ORIGINS env var. Add PAD_SECURE_COOKIES for TLS deployments (sets Secure flag on session/CSRF cookies and enables HSTS). Also adds X-CSRF-Token to CORS allowed headers. * fix: address PR review — lazy router init and trusted IP for rate limits Fix two issues flagged by Codex: 1. CORS/HSTS config was ignored because setupRouter() ran in New() before SetCORSOrigins/SetSecureCookies were called. Now uses sync.Once to lazily build the router on first ServeHTTP/Listen. 2. Rate limiter read X-Real-IP directly from untrusted headers, allowing clients to spoof IPs. Now uses RemoteAddr only (which chimiddleware.RealIP already sanitizes from trusted proxy headers). --- cmd/pad/main.go | 2 + go.mod | 1 + go.sum | 2 + internal/config/config.go | 10 + internal/server/handlers_activity.go | 4 +- internal/server/handlers_agent_roles.go | 19 +- internal/server/handlers_auth.go | 21 +- internal/server/handlers_auth_test.go | 7 + internal/server/handlers_collections.go | 23 +- internal/server/handlers_comments.go | 29 +- internal/server/handlers_dashboard.go | 4 +- internal/server/handlers_documents.go | 33 +- internal/server/handlers_events.go | 2 +- internal/server/handlers_item_links.go | 20 +- internal/server/handlers_item_versions.go | 13 +- internal/server/handlers_items.go | 61 ++- internal/server/handlers_members.go | 18 +- internal/server/handlers_rbac_test.go | 433 +++++++++++++++++++ internal/server/handlers_role_board.go | 12 +- internal/server/handlers_search.go | 27 +- internal/server/handlers_timeline.go | 6 +- internal/server/handlers_tokens.go | 12 +- internal/server/handlers_versions.go | 6 +- internal/server/handlers_views.go | 21 +- internal/server/handlers_webhooks.go | 24 +- internal/server/handlers_workspaces.go | 19 +- internal/server/middleware_auth.go | 10 + internal/server/middleware_csrf.go | 117 +++++ internal/server/middleware_csrf_test.go | 249 +++++++++++ internal/server/middleware_ratelimit.go | 205 +++++++++ internal/server/middleware_ratelimit_test.go | 148 +++++++ internal/server/middleware_security.go | 62 +++ internal/server/middleware_security_test.go | 66 +++ internal/server/server.go | 77 +++- internal/store/search.go | 24 +- internal/webhooks/dispatcher.go | 14 +- internal/webhooks/dispatcher_test.go | 5 + internal/webhooks/validate.go | 107 +++++ internal/webhooks/validate_test.go | 86 ++++ web/src/lib/api/client.ts | 17 +- 40 files changed, 1880 insertions(+), 136 deletions(-) create mode 100644 internal/server/handlers_rbac_test.go create mode 100644 internal/server/middleware_csrf.go create mode 100644 internal/server/middleware_csrf_test.go create mode 100644 internal/server/middleware_ratelimit.go create mode 100644 internal/server/middleware_ratelimit_test.go create mode 100644 internal/server/middleware_security.go create mode 100644 internal/server/middleware_security_test.go create mode 100644 internal/webhooks/validate.go create mode 100644 internal/webhooks/validate_test.go diff --git a/cmd/pad/main.go b/cmd/pad/main.go index 97e3ca09..e2dc0cd9 100644 --- a/cmd/pad/main.go +++ b/cmd/pad/main.go @@ -191,6 +191,8 @@ func serveCmd() *cobra.Command { srv := server.New(s) srv.SetVersion(version, commit, buildTime) srv.SetBaseURL(cfg.BaseURL()) + srv.SetCORSOrigins(cfg.CORSOrigins) + srv.SetSecureCookies(cfg.SecureCookies) // Attach event bus for real-time SSE srv.SetEventBus(events.New()) diff --git a/go.mod b/go.mod index b736bffb..d6694577 100644 --- a/go.mod +++ b/go.mod @@ -20,6 +20,7 @@ require ( golang.org/x/crypto v0.49.0 // indirect golang.org/x/sys v0.42.0 // indirect golang.org/x/term v0.41.0 // indirect + golang.org/x/time v0.15.0 // indirect modernc.org/libc v1.70.0 // indirect modernc.org/mathutil v1.7.1 // indirect modernc.org/memory v1.11.0 // indirect diff --git a/go.sum b/go.sum index ca3c883a..6afcc6d5 100644 --- a/go.sum +++ b/go.sum @@ -44,6 +44,8 @@ golang.org/x/sys v0.42.0 h1:omrd2nAlyT5ESRdCLYdm3+fMfNFE/+Rf4bDIQImRJeo= golang.org/x/sys v0.42.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= golang.org/x/term v0.41.0 h1:QCgPso/Q3RTJx2Th4bDLqML4W6iJiaXFq2/ftQF13YU= golang.org/x/term v0.41.0/go.mod h1:3pfBgksrReYfZ5lvYM0kSO0LIkAl4Yl2bXOkKP7Ec2A= +golang.org/x/time v0.15.0 h1:bbrp8t3bGUeFOx08pvsMYRTCVSMk89u4tKbNOZbp88U= +golang.org/x/time v0.15.0/go.mod h1:Y4YMaQmXwGQZoFaVFk4YpCt4FLQMYKZe9oeV/f4MSno= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= diff --git a/internal/config/config.go b/internal/config/config.go index 0b59811e..05662dc0 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -36,6 +36,10 @@ type Config struct { MailerooAPIKey string `toml:"maileroo_api_key"` EmailFrom string `toml:"email_from"` // Sender address (e.g. noreply@getpad.dev) EmailFromName string `toml:"email_from_name"` // Sender display name (e.g. Pad) + + // Security + CORSOrigins string `toml:"cors_origins"` // Comma-separated allowed origins (e.g. "https://app.pad.dev,https://admin.pad.dev") + SecureCookies bool `toml:"secure_cookies"` // Set Secure flag on cookies (requires TLS) } func DefaultConfig() *Config { @@ -118,6 +122,12 @@ func Load() (*Config, error) { if v := os.Getenv("PAD_EMAIL_FROM_NAME"); v != "" { cfg.EmailFromName = v } + if v := os.Getenv("PAD_CORS_ORIGINS"); v != "" { + cfg.CORSOrigins = v + } + if v := os.Getenv("PAD_SECURE_COOKIES"); v == "true" || v == "1" { + cfg.SecureCookies = true + } return cfg, nil } diff --git a/internal/server/handlers_activity.go b/internal/server/handlers_activity.go index 3d100b74..2e7b0ea2 100644 --- a/internal/server/handlers_activity.go +++ b/internal/server/handlers_activity.go @@ -32,7 +32,7 @@ func (s *Server) handleListWorkspaceActivity(w http.ResponseWriter, r *http.Requ activities, err := s.store.ListWorkspaceActivity(workspaceID, params) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if activities == nil { @@ -69,7 +69,7 @@ func (s *Server) handleListDocumentActivity(w http.ResponseWriter, r *http.Reque activities, err := s.store.ListDocumentActivity(doc.ID, params) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if activities == nil { diff --git a/internal/server/handlers_agent_roles.go b/internal/server/handlers_agent_roles.go index 73637491..c118c854 100644 --- a/internal/server/handlers_agent_roles.go +++ b/internal/server/handlers_agent_roles.go @@ -18,7 +18,7 @@ func (s *Server) handleListAgentRoles(w http.ResponseWriter, r *http.Request) { roles, err := s.store.ListAgentRoles(workspaceID) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } @@ -27,6 +27,9 @@ func (s *Server) handleListAgentRoles(w http.ResponseWriter, r *http.Request) { // handleCreateAgentRole creates a new agent role in a workspace. func (s *Server) handleCreateAgentRole(w http.ResponseWriter, r *http.Request) { + if !requireMinRole(w, r, "owner") { + return + } workspaceID, ok := s.getWorkspaceID(w, r) if !ok { return @@ -45,7 +48,7 @@ func (s *Server) handleCreateAgentRole(w http.ResponseWriter, r *http.Request) { role, err := s.store.CreateAgentRole(workspaceID, input) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } @@ -62,7 +65,7 @@ func (s *Server) handleGetAgentRole(w http.ResponseWriter, r *http.Request) { roleID := chi.URLParam(r, "roleID") role, err := s.store.GetAgentRole(workspaceID, roleID) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if role == nil { @@ -75,6 +78,9 @@ func (s *Server) handleGetAgentRole(w http.ResponseWriter, r *http.Request) { // handleUpdateAgentRole updates an existing agent role. func (s *Server) handleUpdateAgentRole(w http.ResponseWriter, r *http.Request) { + if !requireMinRole(w, r, "owner") { + return + } workspaceID, ok := s.getWorkspaceID(w, r) if !ok { return @@ -89,7 +95,7 @@ func (s *Server) handleUpdateAgentRole(w http.ResponseWriter, r *http.Request) { role, err := s.store.UpdateAgentRole(workspaceID, roleID, input) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if role == nil { @@ -102,6 +108,9 @@ func (s *Server) handleUpdateAgentRole(w http.ResponseWriter, r *http.Request) { // handleDeleteAgentRole removes an agent role from a workspace. func (s *Server) handleDeleteAgentRole(w http.ResponseWriter, r *http.Request) { + if !requireMinRole(w, r, "owner") { + return + } workspaceID, ok := s.getWorkspaceID(w, r) if !ok { return @@ -113,7 +122,7 @@ func (s *Server) handleDeleteAgentRole(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusNotFound, "not_found", "Agent role not found") return } - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } diff --git a/internal/server/handlers_auth.go b/internal/server/handlers_auth.go index 999b5a44..ba920b82 100644 --- a/internal/server/handlers_auth.go +++ b/internal/server/handlers_auth.go @@ -81,9 +81,13 @@ func (s *Server) createAuthSession(w http.ResponseWriter, user *models.User, ttl Path: "/", MaxAge: int(ttl.Seconds()), HttpOnly: true, + Secure: s.secureCookies, SameSite: http.SameSiteLaxMode, }) + // Set CSRF cookie alongside the session cookie + setCSRFCookie(w, int(ttl.Seconds()), s.secureCookies) + return token, nil } @@ -377,9 +381,13 @@ func (s *Server) handleLogout(w http.ResponseWriter, r *http.Request) { Path: "/", MaxAge: -1, HttpOnly: true, + Secure: s.secureCookies, SameSite: http.SameSiteLaxMode, }) + // Clear CSRF cookie on logout + clearCSRFCookie(w) + writeJSON(w, http.StatusOK, map[string]interface{}{ "ok": true, }) @@ -529,7 +537,7 @@ func (s *Server) handleForgotPassword(w http.ResponseWriter, r *http.Request) { // Generate reset token token, err := s.store.CreatePasswordReset(user.ID) if err != nil { - log.Printf("Failed to create password reset for %s: %v", input.Email, err) + log.Printf("Failed to create password reset: %v", err) writeJSON(w, http.StatusOK, okResponse) return } @@ -539,12 +547,11 @@ func (s *Server) handleForgotPassword(w http.ResponseWriter, r *http.Request) { resetURL := s.baseURL + "/reset-password/" + token go func() { if err := s.email.SendPasswordReset(context.Background(), user.Email, user.Name, resetURL); err != nil { - log.Printf("Failed to send password reset email to %s: %v", user.Email, err) + log.Printf("Failed to send password reset email: %v", err) } }() } else { - resetURL := s.baseURL + "/reset-password/" + token - log.Printf("Password reset token generated for %s (email not configured). Reset URL: %s", input.Email, resetURL) + log.Printf("Password reset token generated (email not configured). Use pad auth reset-password to manage.") } writeJSON(w, http.StatusOK, okResponse) @@ -591,7 +598,7 @@ func (s *Server) handleResetPassword(w http.ResponseWriter, r *http.Request) { // Invalidate all existing sessions (force logout everywhere) if err := s.store.DeleteUserSessions(user.ID); err != nil { - log.Printf("Failed to invalidate sessions for user %s after password reset: %v", user.ID, err) + log.Printf("Failed to invalidate sessions after password reset: %v", err) } // Create a fresh session so the user is logged in @@ -607,9 +614,13 @@ func (s *Server) handleResetPassword(w http.ResponseWriter, r *http.Request) { Path: "/", MaxAge: int(webSessionTTL.Seconds()), HttpOnly: true, + Secure: s.secureCookies, SameSite: http.SameSiteLaxMode, }) + // Set CSRF cookie alongside the new session + setCSRFCookie(w, int(webSessionTTL.Seconds()), s.secureCookies) + writeJSON(w, http.StatusOK, map[string]interface{}{ "ok": true, "user": map[string]interface{}{ diff --git a/internal/server/handlers_auth_test.go b/internal/server/handlers_auth_test.go index 5fe1bdb5..72a20efb 100644 --- a/internal/server/handlers_auth_test.go +++ b/internal/server/handlers_auth_test.go @@ -343,6 +343,13 @@ func doRequestWithCookie(srv *Server, method, path string, body interface{}, tok Name: "pad_session", Value: token, }) + // Include CSRF token for the double-submit cookie pattern + const testCSRF = "test-csrf-token" + req.AddCookie(&http.Cookie{ + Name: "pad_csrf", + Value: testCSRF, + }) + req.Header.Set("X-CSRF-Token", testCSRF) rr := httptest.NewRecorder() srv.ServeHTTP(rr, req) return rr diff --git a/internal/server/handlers_collections.go b/internal/server/handlers_collections.go index 41cfba9e..3b242ea2 100644 --- a/internal/server/handlers_collections.go +++ b/internal/server/handlers_collections.go @@ -18,7 +18,7 @@ func (s *Server) handleListCollections(w http.ResponseWriter, r *http.Request) { colls, err := s.store.ListCollections(workspaceID) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if colls == nil { @@ -28,6 +28,9 @@ func (s *Server) handleListCollections(w http.ResponseWriter, r *http.Request) { } func (s *Server) handleCreateCollection(w http.ResponseWriter, r *http.Request) { + if !requireMinRole(w, r, "owner") { + return + } workspaceID, ok := s.getWorkspaceID(w, r) if !ok { return @@ -50,7 +53,7 @@ func (s *Server) handleCreateCollection(w http.ResponseWriter, r *http.Request) writeError(w, http.StatusConflict, "conflict", "A collection with this name already exists") return } - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } @@ -66,7 +69,7 @@ func (s *Server) handleGetCollection(w http.ResponseWriter, r *http.Request) { collSlug := chi.URLParam(r, "collSlug") coll, err := s.store.GetCollectionBySlug(workspaceID, collSlug) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if coll == nil { @@ -78,6 +81,9 @@ func (s *Server) handleGetCollection(w http.ResponseWriter, r *http.Request) { } func (s *Server) handleUpdateCollection(w http.ResponseWriter, r *http.Request) { + if !requireMinRole(w, r, "owner") { + return + } workspaceID, ok := s.getWorkspaceID(w, r) if !ok { return @@ -86,7 +92,7 @@ func (s *Server) handleUpdateCollection(w http.ResponseWriter, r *http.Request) collSlug := chi.URLParam(r, "collSlug") coll, err := s.store.GetCollectionBySlug(workspaceID, collSlug) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if coll == nil { @@ -106,7 +112,7 @@ func (s *Server) handleUpdateCollection(w http.ResponseWriter, r *http.Request) updated, err := s.store.UpdateCollection(coll.ID, input) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if updated == nil { @@ -126,6 +132,9 @@ func (s *Server) handleUpdateCollection(w http.ResponseWriter, r *http.Request) } func (s *Server) handleDeleteCollection(w http.ResponseWriter, r *http.Request) { + if !requireMinRole(w, r, "owner") { + return + } workspaceID, ok := s.getWorkspaceID(w, r) if !ok { return @@ -134,7 +143,7 @@ func (s *Server) handleDeleteCollection(w http.ResponseWriter, r *http.Request) collSlug := chi.URLParam(r, "collSlug") coll, err := s.store.GetCollectionBySlug(workspaceID, collSlug) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if coll == nil { @@ -151,7 +160,7 @@ func (s *Server) handleDeleteCollection(w http.ResponseWriter, r *http.Request) writeError(w, http.StatusBadRequest, "bad_request", "Cannot delete a default collection") return } - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } diff --git a/internal/server/handlers_comments.go b/internal/server/handlers_comments.go index 39274afc..a7917140 100644 --- a/internal/server/handlers_comments.go +++ b/internal/server/handlers_comments.go @@ -22,7 +22,7 @@ func (s *Server) handleListComments(w http.ResponseWriter, r *http.Request) { itemSlug := chi.URLParam(r, "itemSlug") item, err := s.store.ResolveItem(workspaceID, itemSlug) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if item == nil { @@ -32,7 +32,7 @@ func (s *Server) handleListComments(w http.ResponseWriter, r *http.Request) { comments, err := s.store.ListComments(item.ID) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if comments == nil { @@ -60,6 +60,9 @@ func (s *Server) handleListComments(w http.ResponseWriter, r *http.Request) { // handleCreateComment adds a new comment to an item. func (s *Server) handleCreateComment(w http.ResponseWriter, r *http.Request) { + if !requireMinRole(w, r, "editor") { + return + } workspaceID, ok := s.getWorkspaceID(w, r) if !ok { return @@ -68,7 +71,7 @@ func (s *Server) handleCreateComment(w http.ResponseWriter, r *http.Request) { itemSlug := chi.URLParam(r, "itemSlug") item, err := s.store.ResolveItem(workspaceID, itemSlug) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if item == nil { @@ -103,7 +106,7 @@ func (s *Server) handleCreateComment(w http.ResponseWriter, r *http.Request) { comment, err := s.store.CreateComment(workspaceID, item.ID, input) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } @@ -119,6 +122,9 @@ func (s *Server) handleCreateComment(w http.ResponseWriter, r *http.Request) { // handleDeleteComment removes a comment. func (s *Server) handleDeleteComment(w http.ResponseWriter, r *http.Request) { + if !requireMinRole(w, r, "editor") { + return + } workspaceID, ok := s.getWorkspaceID(w, r) if !ok { return @@ -138,7 +144,7 @@ func (s *Server) handleDeleteComment(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusNotFound, "not_found", "Comment not found") return } - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } @@ -147,6 +153,9 @@ func (s *Server) handleDeleteComment(w http.ResponseWriter, r *http.Request) { // handleCreateReply creates a reply to an existing comment. func (s *Server) handleCreateReply(w http.ResponseWriter, r *http.Request) { + if !requireMinRole(w, r, "editor") { + return + } workspaceID, ok := s.getWorkspaceID(w, r) if !ok { return @@ -191,7 +200,7 @@ func (s *Server) handleCreateReply(w http.ResponseWriter, r *http.Request) { comment, err := s.store.CreateComment(workspaceID, parentComment.ItemID, input) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } @@ -202,6 +211,9 @@ func (s *Server) handleCreateReply(w http.ResponseWriter, r *http.Request) { // handleAddReaction adds an emoji reaction to a comment. func (s *Server) handleAddReaction(w http.ResponseWriter, r *http.Request) { + if !requireMinRole(w, r, "editor") { + return + } workspaceID, ok := s.getWorkspaceID(w, r) if !ok { return @@ -233,7 +245,7 @@ func (s *Server) handleAddReaction(w http.ResponseWriter, r *http.Request) { reaction, err := s.store.AddReaction(commentID, userID, actor, input.Emoji) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } @@ -247,6 +259,9 @@ func (s *Server) handleAddReaction(w http.ResponseWriter, r *http.Request) { // handleRemoveReaction removes an emoji reaction from a comment. func (s *Server) handleRemoveReaction(w http.ResponseWriter, r *http.Request) { + if !requireMinRole(w, r, "editor") { + return + } workspaceID, ok := s.getWorkspaceID(w, r) if !ok { return diff --git a/internal/server/handlers_dashboard.go b/internal/server/handlers_dashboard.go index 7c7beb4a..4fc4c808 100644 --- a/internal/server/handlers_dashboard.go +++ b/internal/server/handlers_dashboard.go @@ -137,7 +137,7 @@ func (s *Server) handleGetDashboard(w http.ResponseWriter, r *http.Request) { // Build a schema map for terminal status lookups collections, err := s.store.ListCollections(workspaceID) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } schemaMap := buildSchemaMap(collections) @@ -156,7 +156,7 @@ func (s *Server) handleGetDashboard(w http.ResponseWriter, r *http.Request) { // Summary: items grouped by collection slug and status field allItems, err := s.store.ListItems(workspaceID, models.ItemListParams{}) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } diff --git a/internal/server/handlers_documents.go b/internal/server/handlers_documents.go index 37dd58ed..8f5d0a0c 100644 --- a/internal/server/handlers_documents.go +++ b/internal/server/handlers_documents.go @@ -35,7 +35,7 @@ func (s *Server) handleListDocuments(w http.ResponseWriter, r *http.Request) { docs, err := s.store.ListDocuments(workspaceID, params) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if docs == nil { @@ -45,6 +45,9 @@ func (s *Server) handleListDocuments(w http.ResponseWriter, r *http.Request) { } func (s *Server) handleCreateDocument(w http.ResponseWriter, r *http.Request) { + if !requireMinRole(w, r, "editor") { + return + } workspaceID, ok := s.getWorkspaceID(w, r) if !ok { return @@ -75,7 +78,7 @@ func (s *Server) handleCreateDocument(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusConflict, "conflict", "A document with this title already exists in this workspace") return } - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } @@ -96,6 +99,9 @@ func (s *Server) handleGetDocument(w http.ResponseWriter, r *http.Request) { } func (s *Server) handleUpdateDocument(w http.ResponseWriter, r *http.Request) { + if !requireMinRole(w, r, "editor") { + return + } _, doc, ok := s.getWorkspaceDocument(w, r) if !ok { return @@ -122,7 +128,7 @@ func (s *Server) handleUpdateDocument(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusConflict, "conflict", "A document with this title already exists in this workspace") return } - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if updated == nil { @@ -147,13 +153,16 @@ func (s *Server) handleUpdateDocument(w http.ResponseWriter, r *http.Request) { } func (s *Server) handleDeleteDocument(w http.ResponseWriter, r *http.Request) { + if !requireMinRole(w, r, "editor") { + return + } _, doc, ok := s.getWorkspaceDocument(w, r) if !ok { return } if err := s.store.DeleteDocument(doc.ID); err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } @@ -165,6 +174,9 @@ func (s *Server) handleDeleteDocument(w http.ResponseWriter, r *http.Request) { } func (s *Server) handleRestoreDocument(w http.ResponseWriter, r *http.Request) { + if !requireMinRole(w, r, "editor") { + return + } // Restore needs special handling — doc is soft-deleted so getWorkspaceDocument won't find it. // Verify workspace exists, then restore by ID. workspaceID, ok := s.getWorkspaceID(w, r) @@ -195,6 +207,9 @@ func (s *Server) handleRestoreDocument(w http.ResponseWriter, r *http.Request) { } func (s *Server) handleQuickSave(w http.ResponseWriter, r *http.Request) { + if !requireMinRole(w, r, "editor") { + return + } workspaceID, ok := s.getWorkspaceID(w, r) if !ok { return @@ -229,7 +244,7 @@ func (s *Server) handleQuickSave(w http.ResponseWriter, r *http.Request) { doc, err := s.store.QuickSave(workspaceID, input) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } @@ -258,7 +273,7 @@ func (s *Server) handleBulkRead(w http.ResponseWriter, r *http.Request) { docs, err := s.store.BulkRead(input.IDs) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if docs == nil { @@ -275,7 +290,7 @@ func (s *Server) handleGetBacklinks(w http.ResponseWriter, r *http.Request) { backlinks, err := s.store.GetBacklinks(workspaceID, doc.Title) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if backlinks == nil { @@ -303,7 +318,7 @@ func (s *Server) handleGetLinks(w http.ResponseWriter, r *http.Request) { } linkedDocs, err := s.store.GetLinks(workspaceID, doc.Content) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if linkedDocs == nil { @@ -331,7 +346,7 @@ func (s *Server) handleGetContext(w http.ResponseWriter, r *http.Request) { docs, err := s.store.GetContext(workspaceID, types, includeContent) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if docs == nil { diff --git a/internal/server/handlers_events.go b/internal/server/handlers_events.go index 3cc031c6..4dbcad21 100644 --- a/internal/server/handlers_events.go +++ b/internal/server/handlers_events.go @@ -26,7 +26,7 @@ func (s *Server) handleSSE(w http.ResponseWriter, r *http.Request) { ws, err := s.store.GetWorkspaceBySlug(slug) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if ws == nil { diff --git a/internal/server/handlers_item_links.go b/internal/server/handlers_item_links.go index fc6f2b59..fc083baf 100644 --- a/internal/server/handlers_item_links.go +++ b/internal/server/handlers_item_links.go @@ -20,7 +20,7 @@ func (s *Server) handleGetItemLinks(w http.ResponseWriter, r *http.Request) { itemSlug := chi.URLParam(r, "itemSlug") item, err := s.store.ResolveItem(workspaceID, itemSlug) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if item == nil { @@ -30,7 +30,7 @@ func (s *Server) handleGetItemLinks(w http.ResponseWriter, r *http.Request) { links, err := s.store.GetItemLinks(item.ID) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if links == nil { @@ -42,6 +42,9 @@ func (s *Server) handleGetItemLinks(w http.ResponseWriter, r *http.Request) { // handleCreateItemLink creates a new link between two items. func (s *Server) handleCreateItemLink(w http.ResponseWriter, r *http.Request) { + if !requireMinRole(w, r, "editor") { + return + } workspaceID, ok := s.getWorkspaceID(w, r) if !ok { return @@ -50,7 +53,7 @@ func (s *Server) handleCreateItemLink(w http.ResponseWriter, r *http.Request) { itemSlug := chi.URLParam(r, "itemSlug") item, err := s.store.ResolveItem(workspaceID, itemSlug) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if item == nil { @@ -79,7 +82,7 @@ func (s *Server) handleCreateItemLink(w http.ResponseWriter, r *http.Request) { // Verify target item exists target, err := s.store.GetItem(input.TargetID) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if target == nil || target.WorkspaceID != workspaceID { @@ -97,7 +100,7 @@ func (s *Server) handleCreateItemLink(w http.ResponseWriter, r *http.Request) { actor, _ := actorFromRequest(r) link, err := s.store.SetPhaseLink(workspaceID, item.ID, target.ID, actor) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } writeJSON(w, http.StatusCreated, link) @@ -114,7 +117,7 @@ func (s *Server) handleCreateItemLink(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusBadRequest, "bad_request", err.Error()) return } - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } @@ -123,6 +126,9 @@ func (s *Server) handleCreateItemLink(w http.ResponseWriter, r *http.Request) { // handleDeleteItemLink removes a link between items. func (s *Server) handleDeleteItemLink(w http.ResponseWriter, r *http.Request) { + if !requireMinRole(w, r, "editor") { + return + } _, ok := s.getWorkspaceID(w, r) if !ok { return @@ -134,7 +140,7 @@ func (s *Server) handleDeleteItemLink(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusNotFound, "not_found", "Link not found") return } - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } diff --git a/internal/server/handlers_item_versions.go b/internal/server/handlers_item_versions.go index 8ede9723..7257ccd1 100644 --- a/internal/server/handlers_item_versions.go +++ b/internal/server/handlers_item_versions.go @@ -19,7 +19,7 @@ func (s *Server) handleListItemVersions(w http.ResponseWriter, r *http.Request) itemSlug := chi.URLParam(r, "itemSlug") item, err := s.store.ResolveItem(workspaceID, itemSlug) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if item == nil { @@ -29,7 +29,7 @@ func (s *Server) handleListItemVersions(w http.ResponseWriter, r *http.Request) versions, err := s.store.ListItemVersionsResolved(item.ID, item.Content) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if versions == nil { @@ -41,6 +41,9 @@ func (s *Server) handleListItemVersions(w http.ResponseWriter, r *http.Request) // handleRestoreItemVersion restores an item's content from a specific version. func (s *Server) handleRestoreItemVersion(w http.ResponseWriter, r *http.Request) { + if !requireMinRole(w, r, "editor") { + return + } workspaceID, ok := s.getWorkspaceID(w, r) if !ok { return @@ -51,7 +54,7 @@ func (s *Server) handleRestoreItemVersion(w http.ResponseWriter, r *http.Request item, err := s.store.ResolveItem(workspaceID, itemSlug) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if item == nil { @@ -62,7 +65,7 @@ func (s *Server) handleRestoreItemVersion(w http.ResponseWriter, r *http.Request // Get all resolved versions to find the target versions, err := s.store.ListItemVersionsResolved(item.ID, item.Content) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } @@ -90,7 +93,7 @@ func (s *Server) handleRestoreItemVersion(w http.ResponseWriter, r *http.Request updated, err := s.store.UpdateItem(item.ID, input) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } diff --git a/internal/server/handlers_items.go b/internal/server/handlers_items.go index cd5345f2..6253644b 100644 --- a/internal/server/handlers_items.go +++ b/internal/server/handlers_items.go @@ -31,7 +31,7 @@ func (s *Server) handleListItems(w http.ResponseWriter, r *http.Request) { } result, err := s.store.ListItems(workspaceID, params) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if result == nil { @@ -52,7 +52,7 @@ func (s *Server) handleListCollectionItems(w http.ResponseWriter, r *http.Reques collSlug := chi.URLParam(r, "collSlug") coll, err := s.store.GetCollectionBySlug(workspaceID, collSlug) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if coll == nil { @@ -69,7 +69,7 @@ func (s *Server) handleListCollectionItems(w http.ResponseWriter, r *http.Reques result, err := s.store.ListItems(workspaceID, params) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if result == nil { @@ -82,6 +82,9 @@ func (s *Server) handleListCollectionItems(w http.ResponseWriter, r *http.Reques // handleCreateItem creates a new item in a collection, validating fields against the schema. func (s *Server) handleCreateItem(w http.ResponseWriter, r *http.Request) { + if !requireMinRole(w, r, "editor") { + return + } workspaceID, ok := s.getWorkspaceID(w, r) if !ok { return @@ -90,7 +93,7 @@ func (s *Server) handleCreateItem(w http.ResponseWriter, r *http.Request) { collSlug := chi.URLParam(r, "collSlug") coll, err := s.store.GetCollectionBySlug(workspaceID, collSlug) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if coll == nil { @@ -163,7 +166,7 @@ func (s *Server) handleCreateItem(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusConflict, "conflict", "An item with this title already exists") return } - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } @@ -182,7 +185,7 @@ func (s *Server) handleCreateItem(w http.ResponseWriter, r *http.Request) { s.dispatchWebhook(workspaceID, "item.created", item) if err := s.enrichItemForResponse(item); err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } @@ -199,7 +202,7 @@ func (s *Server) handleGetItem(w http.ResponseWriter, r *http.Request) { itemSlug := chi.URLParam(r, "itemSlug") item, err := s.store.ResolveItem(workspaceID, itemSlug) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if item == nil { @@ -208,7 +211,7 @@ func (s *Server) handleGetItem(w http.ResponseWriter, r *http.Request) { } if err := s.enrichItemForResponse(item); err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } @@ -217,6 +220,9 @@ func (s *Server) handleGetItem(w http.ResponseWriter, r *http.Request) { // handleUpdateItem updates an existing item (fields, content, or both). func (s *Server) handleUpdateItem(w http.ResponseWriter, r *http.Request) { + if !requireMinRole(w, r, "editor") { + return + } workspaceID, ok := s.getWorkspaceID(w, r) if !ok { return @@ -225,7 +231,7 @@ func (s *Server) handleUpdateItem(w http.ResponseWriter, r *http.Request) { itemSlug := chi.URLParam(r, "itemSlug") item, err := s.store.ResolveItem(workspaceID, itemSlug) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if item == nil { @@ -317,7 +323,7 @@ func (s *Server) handleUpdateItem(w http.ResponseWriter, r *http.Request) { updated, err := s.store.UpdateItem(item.ID, input) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if updated == nil { @@ -376,7 +382,7 @@ func (s *Server) handleUpdateItem(w http.ResponseWriter, r *http.Request) { } if err := s.enrichItemForResponse(updated); err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } @@ -385,6 +391,9 @@ func (s *Server) handleUpdateItem(w http.ResponseWriter, r *http.Request) { // handleDeleteItem archives (soft-deletes) an item. func (s *Server) handleDeleteItem(w http.ResponseWriter, r *http.Request) { + if !requireMinRole(w, r, "editor") { + return + } workspaceID, ok := s.getWorkspaceID(w, r) if !ok { return @@ -393,7 +402,7 @@ func (s *Server) handleDeleteItem(w http.ResponseWriter, r *http.Request) { itemSlug := chi.URLParam(r, "itemSlug") item, err := s.store.ResolveItem(workspaceID, itemSlug) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if item == nil { @@ -402,7 +411,7 @@ func (s *Server) handleDeleteItem(w http.ResponseWriter, r *http.Request) { } if err := s.store.DeleteItem(item.ID); err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } @@ -416,6 +425,9 @@ func (s *Server) handleDeleteItem(w http.ResponseWriter, r *http.Request) { // handleRestoreItem restores an archived item. func (s *Server) handleRestoreItem(w http.ResponseWriter, r *http.Request) { + if !requireMinRole(w, r, "editor") { + return + } workspaceID, ok := s.getWorkspaceID(w, r) if !ok { return @@ -426,7 +438,7 @@ func (s *Server) handleRestoreItem(w http.ResponseWriter, r *http.Request) { // We need to find the item even if deleted (for restore). item, err := s.store.ResolveItemIncludeDeleted(workspaceID, itemSlug) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if item == nil { @@ -440,7 +452,7 @@ func (s *Server) handleRestoreItem(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusNotFound, "not_found", "Item not found or not archived") return } - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } @@ -449,7 +461,7 @@ func (s *Server) handleRestoreItem(w http.ResponseWriter, r *http.Request) { s.publishItemEventWithName(events.ItemRestored, workspaceID, restored.ID, restored.Title, restored.CollectionSlug, actor, actorNameFromRequest(r), source) if err := s.enrichItemForResponse(restored); err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } @@ -458,6 +470,9 @@ func (s *Server) handleRestoreItem(w http.ResponseWriter, r *http.Request) { // handleMoveItem moves an item to a different collection with field migration. func (s *Server) handleMoveItem(w http.ResponseWriter, r *http.Request) { + if !requireMinRole(w, r, "editor") { + return + } workspaceID, ok := s.getWorkspaceID(w, r) if !ok { return @@ -545,7 +560,7 @@ func (s *Server) handleMoveItem(w http.ResponseWriter, r *http.Request) { // Move the item moved, err := s.store.MoveItem(item.ID, targetColl.ID, string(fieldsJSON)) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } @@ -559,7 +574,7 @@ func (s *Server) handleMoveItem(w http.ResponseWriter, r *http.Request) { s.dispatchWebhook(workspaceID, "item.moved", moved) if err := s.enrichItemForResponse(moved); err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } @@ -597,7 +612,7 @@ func (s *Server) handlePhasesProgress(w http.ResponseWriter, r *http.Request) { progress, err := s.store.GetAllPhasesProgress(workspaceID) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } writeJSON(w, http.StatusOK, progress) @@ -613,7 +628,7 @@ func (s *Server) handleGetItemTasks(w http.ResponseWriter, r *http.Request) { itemSlug := chi.URLParam(r, "itemSlug") item, err := s.store.ResolveItem(workspaceID, itemSlug) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if item == nil { @@ -623,7 +638,7 @@ func (s *Server) handleGetItemTasks(w http.ResponseWriter, r *http.Request) { tasks, err := s.store.GetTasksForPhase(item.ID) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if tasks == nil { @@ -926,7 +941,7 @@ func (s *Server) handleListItemActivity(w http.ResponseWriter, r *http.Request) itemSlug := chi.URLParam(r, "itemSlug") item, err := s.store.ResolveItem(workspaceID, itemSlug) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if item == nil { @@ -947,7 +962,7 @@ func (s *Server) handleListItemActivity(w http.ResponseWriter, r *http.Request) activities, err := s.store.ListDocumentActivity(item.ID, params) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if activities == nil { diff --git a/internal/server/handlers_members.go b/internal/server/handlers_members.go index 5f14bf4d..0fb6e2e8 100644 --- a/internal/server/handlers_members.go +++ b/internal/server/handlers_members.go @@ -17,14 +17,14 @@ func (s *Server) handleListMembers(w http.ResponseWriter, r *http.Request) { members, err := s.store.ListWorkspaceMembers(workspaceID) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } // Include pending invitations, enriched with join URLs invitations, err := s.store.ListWorkspaceInvitations(workspaceID) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } @@ -91,7 +91,7 @@ func (s *Server) handleInviteMember(w http.ResponseWriter, r *http.Request) { // Check if user with this email already exists existingUser, err := s.store.GetUserByEmail(input.Email) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } @@ -103,7 +103,7 @@ func (s *Server) handleInviteMember(w http.ResponseWriter, r *http.Request) { return } if err := s.store.AddWorkspaceMember(workspaceID, existingUser.ID, input.Role); err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } writeJSON(w, http.StatusCreated, map[string]interface{}{ @@ -119,7 +119,7 @@ func (s *Server) handleInviteMember(w http.ResponseWriter, r *http.Request) { // User doesn't exist — create an invitation inv, err := s.store.CreateInvitation(workspaceID, input.Email, input.Role, inviterID) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } @@ -149,7 +149,7 @@ func (s *Server) handleInviteMember(w http.ResponseWriter, r *http.Request) { wsName = ws.Name } if err := s.email.SendInvitation(context.Background(), inv.Email, inviterName, wsName, joinURL); err != nil { - log.Printf("Failed to send invitation email to %s: %v", inv.Email, err) + log.Printf("Failed to send invitation email: %v", err) } }() } @@ -248,7 +248,7 @@ func (s *Server) handleAcceptInvitation(w http.ResponseWriter, r *http.Request) inv, err := s.store.GetInvitationByCode(code) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if inv == nil { @@ -264,13 +264,13 @@ func (s *Server) handleAcceptInvitation(w http.ResponseWriter, r *http.Request) // Add user to workspace if err := s.store.AddWorkspaceMember(inv.WorkspaceID, user.ID, inv.Role); err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } // Mark invitation as accepted if err := s.store.AcceptInvitation(inv.ID); err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } diff --git a/internal/server/handlers_rbac_test.go b/internal/server/handlers_rbac_test.go new file mode 100644 index 00000000..288d8256 --- /dev/null +++ b/internal/server/handlers_rbac_test.go @@ -0,0 +1,433 @@ +package server + +import ( + "bytes" + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "testing" + + "github.com/xarmian/pad/internal/models" +) + +// rbacTestEnv holds everything needed for RBAC tests: +// a server with an admin, workspace, and users with different roles. +type rbacTestEnv struct { + srv *Server + wsSlug string + ownerToken string + editorToken string + viewerToken string +} + +func setupRBACEnv(t *testing.T) *rbacTestEnv { + t.Helper() + srv := testServer(t) + + // Bootstrap admin user + ownerToken := bootstrapFirstUser(t, srv, "owner@test.com", "Owner") + + // Create workspace + rr := doRequestWithCookie(srv, "POST", "/api/v1/workspaces", map[string]string{ + "name": "RBAC Test", + }, ownerToken) + if rr.Code != http.StatusCreated { + t.Fatalf("create workspace: expected 201, got %d: %s", rr.Code, rr.Body.String()) + } + var ws models.Workspace + parseJSON(t, rr, &ws) + + // Register editor user + rr = doRequestWithCookie(srv, "POST", "/api/v1/auth/register", map[string]string{ + "email": "editor@test.com", + "name": "Editor", + "password": "password123", + }, ownerToken) + if rr.Code != http.StatusCreated { + t.Fatalf("register editor: expected 201, got %d: %s", rr.Code, rr.Body.String()) + } + + // Register viewer user + rr = doRequestWithCookie(srv, "POST", "/api/v1/auth/register", map[string]string{ + "email": "viewer@test.com", + "name": "Viewer", + "password": "password123", + }, ownerToken) + if rr.Code != http.StatusCreated { + t.Fatalf("register viewer: expected 201, got %d: %s", rr.Code, rr.Body.String()) + } + + // Look up users and add to workspace with roles + editorUser, err := srv.store.GetUserByEmail("editor@test.com") + if err != nil || editorUser == nil { + t.Fatal("failed to find editor user") + } + viewerUser, err := srv.store.GetUserByEmail("viewer@test.com") + if err != nil || viewerUser == nil { + t.Fatal("failed to find viewer user") + } + + if err := srv.store.AddWorkspaceMember(ws.ID, editorUser.ID, "editor"); err != nil { + t.Fatalf("add editor member: %v", err) + } + if err := srv.store.AddWorkspaceMember(ws.ID, viewerUser.ID, "viewer"); err != nil { + t.Fatalf("add viewer member: %v", err) + } + + // Log in as editor + editorToken := loginUser(t, srv, "editor@test.com", "password123") + // Log in as viewer + viewerToken := loginUser(t, srv, "viewer@test.com", "password123") + + return &rbacTestEnv{ + srv: srv, + wsSlug: ws.Slug, + ownerToken: ownerToken, + editorToken: editorToken, + viewerToken: viewerToken, + } +} + +func loginUser(t *testing.T, srv *Server, email, password string) string { + t.Helper() + var bodyReader io.Reader + data, _ := json.Marshal(map[string]string{"email": email, "password": password}) + bodyReader = bytes.NewReader(data) + req := httptest.NewRequest("POST", "/api/v1/auth/login", bodyReader) + req.Header.Set("Content-Type", "application/json") + req.RemoteAddr = "192.0.2.1:1234" + rr := httptest.NewRecorder() + srv.ServeHTTP(rr, req) + if rr.Code != http.StatusOK { + t.Fatalf("login %s: expected 200, got %d: %s", email, rr.Code, rr.Body.String()) + } + var resp map[string]interface{} + parseJSON(t, rr, &resp) + token, _ := resp["token"].(string) + if token == "" { + t.Fatalf("login %s: no token in response", email) + } + return token +} + +func TestRBAC_ViewerBlockedFromItemMutations(t *testing.T) { + env := setupRBACEnv(t) + + // Create an item as owner first + rr := doRequestWithCookie(env.srv, "POST", "/api/v1/workspaces/"+env.wsSlug+"/collections/docs/items", map[string]interface{}{ + "title": "Test Item", + "content": "Content", + }, env.ownerToken) + if rr.Code != http.StatusCreated { + t.Fatalf("setup: create item failed: %d %s", rr.Code, rr.Body.String()) + } + var item map[string]interface{} + parseJSON(t, rr, &item) + itemSlug := item["slug"].(string) + + tests := []struct { + name string + method string + path string + body interface{} + }{ + {"create item", "POST", "/api/v1/workspaces/" + env.wsSlug + "/collections/docs/items", + map[string]interface{}{"title": "New Item"}}, + {"update item", "PATCH", "/api/v1/workspaces/" + env.wsSlug + "/items/" + itemSlug, + map[string]interface{}{"title": "Updated"}}, + {"delete item", "DELETE", "/api/v1/workspaces/" + env.wsSlug + "/items/" + itemSlug, nil}, + {"restore item", "POST", "/api/v1/workspaces/" + env.wsSlug + "/items/" + itemSlug + "/restore", nil}, + {"move item", "POST", "/api/v1/workspaces/" + env.wsSlug + "/items/" + itemSlug + "/move", + map[string]interface{}{"collection_slug": "ideas"}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + rr := doRequestWithCookie(env.srv, tt.method, tt.path, tt.body, env.viewerToken) + if rr.Code != http.StatusForbidden { + t.Errorf("expected 403 for viewer %s, got %d: %s", tt.name, rr.Code, rr.Body.String()) + } + }) + } +} + +func TestRBAC_EditorAllowedItemMutations(t *testing.T) { + env := setupRBACEnv(t) + + // Editor can create items + rr := doRequestWithCookie(env.srv, "POST", "/api/v1/workspaces/"+env.wsSlug+"/collections/docs/items", map[string]interface{}{ + "title": "Editor Item", + "content": "Content", + }, env.editorToken) + if rr.Code != http.StatusCreated { + t.Errorf("expected 201 for editor create item, got %d: %s", rr.Code, rr.Body.String()) + } + + var item map[string]interface{} + parseJSON(t, rr, &item) + itemSlug := item["slug"].(string) + + // Editor can update items (update content, not title, to preserve slug) + rr = doRequestWithCookie(env.srv, "PATCH", "/api/v1/workspaces/"+env.wsSlug+"/items/"+itemSlug, map[string]interface{}{ + "content": "Updated by editor", + }, env.editorToken) + if rr.Code != http.StatusOK { + t.Errorf("expected 200 for editor update item, got %d: %s", rr.Code, rr.Body.String()) + } + + // Editor can delete items + rr = doRequestWithCookie(env.srv, "DELETE", "/api/v1/workspaces/"+env.wsSlug+"/items/"+itemSlug, nil, env.editorToken) + if rr.Code != http.StatusNoContent { + t.Errorf("expected 204 for editor delete item, got %d: %s", rr.Code, rr.Body.String()) + } +} + +func TestRBAC_EditorBlockedFromOwnerOperations(t *testing.T) { + env := setupRBACEnv(t) + + tests := []struct { + name string + method string + path string + body interface{} + }{ + {"create collection", "POST", "/api/v1/workspaces/" + env.wsSlug + "/collections", + map[string]interface{}{"name": "Custom", "schema": `{"fields":[]}`}}, + {"update workspace", "PATCH", "/api/v1/workspaces/" + env.wsSlug, + map[string]interface{}{"name": "Updated"}}, + {"delete workspace", "DELETE", "/api/v1/workspaces/" + env.wsSlug, nil}, + {"export workspace", "GET", "/api/v1/workspaces/" + env.wsSlug + "/export", nil}, + {"create webhook", "POST", "/api/v1/workspaces/" + env.wsSlug + "/webhooks", + map[string]interface{}{"url": "http://example.com", "events": []string{"item.created"}}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + rr := doRequestWithCookie(env.srv, tt.method, tt.path, tt.body, env.editorToken) + if rr.Code != http.StatusForbidden { + t.Errorf("expected 403 for editor %s, got %d: %s", tt.name, rr.Code, rr.Body.String()) + } + }) + } +} + +func TestRBAC_OwnerAllowedEverything(t *testing.T) { + env := setupRBACEnv(t) + + // Owner can create items + rr := doRequestWithCookie(env.srv, "POST", "/api/v1/workspaces/"+env.wsSlug+"/collections/docs/items", map[string]interface{}{ + "title": "Owner Item", + "content": "Content", + }, env.ownerToken) + if rr.Code != http.StatusCreated { + t.Errorf("expected 201 for owner create item, got %d: %s", rr.Code, rr.Body.String()) + } + + // Owner can create collections + rr = doRequestWithCookie(env.srv, "POST", "/api/v1/workspaces/"+env.wsSlug+"/collections", map[string]interface{}{ + "name": "Custom", + "schema": `{"fields":[]}`, + }, env.ownerToken) + if rr.Code != http.StatusCreated { + t.Errorf("expected 201 for owner create collection, got %d: %s", rr.Code, rr.Body.String()) + } + + // Owner can update workspace + rr = doRequestWithCookie(env.srv, "PATCH", "/api/v1/workspaces/"+env.wsSlug, map[string]interface{}{ + "name": "Updated by Owner", + }, env.ownerToken) + if rr.Code != http.StatusOK { + t.Errorf("expected 200 for owner update workspace, got %d: %s", rr.Code, rr.Body.String()) + } +} + +func TestRBAC_ViewerBlockedFromDocumentMutations(t *testing.T) { + env := setupRBACEnv(t) + + // Create a doc as owner + rr := doRequestWithCookie(env.srv, "POST", "/api/v1/workspaces/"+env.wsSlug+"/documents", map[string]interface{}{ + "title": "Test Doc", + "content": "Content", + }, env.ownerToken) + if rr.Code != http.StatusCreated { + t.Fatalf("setup: create doc failed: %d %s", rr.Code, rr.Body.String()) + } + var doc map[string]interface{} + parseJSON(t, rr, &doc) + docID := doc["id"].(string) + + tests := []struct { + name string + method string + path string + body interface{} + }{ + {"create document", "POST", "/api/v1/workspaces/" + env.wsSlug + "/documents", + map[string]interface{}{"title": "New Doc"}}, + {"update document", "PATCH", "/api/v1/workspaces/" + env.wsSlug + "/documents/" + docID, + map[string]interface{}{"content": "Updated"}}, + {"delete document", "DELETE", "/api/v1/workspaces/" + env.wsSlug + "/documents/" + docID, nil}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + rr := doRequestWithCookie(env.srv, tt.method, tt.path, tt.body, env.viewerToken) + if rr.Code != http.StatusForbidden { + t.Errorf("expected 403 for viewer %s, got %d: %s", tt.name, rr.Code, rr.Body.String()) + } + }) + } +} + +func TestRBAC_ViewerBlockedFromCommentCreation(t *testing.T) { + env := setupRBACEnv(t) + + // Create item as owner + rr := doRequestWithCookie(env.srv, "POST", "/api/v1/workspaces/"+env.wsSlug+"/collections/docs/items", map[string]interface{}{ + "title": "Commented Item", + }, env.ownerToken) + if rr.Code != http.StatusCreated { + t.Fatalf("setup: create item failed: %d %s", rr.Code, rr.Body.String()) + } + var item map[string]interface{} + parseJSON(t, rr, &item) + itemSlug := item["slug"].(string) + + rr = doRequestWithCookie(env.srv, "POST", "/api/v1/workspaces/"+env.wsSlug+"/items/"+itemSlug+"/comments", map[string]interface{}{ + "body": "Hello from viewer", + }, env.viewerToken) + if rr.Code != http.StatusForbidden { + t.Errorf("expected 403 for viewer create comment, got %d: %s", rr.Code, rr.Body.String()) + } +} + +func TestRBAC_ViewerCanReadEverything(t *testing.T) { + env := setupRBACEnv(t) + + // Create items/data as owner + doRequestWithCookie(env.srv, "POST", "/api/v1/workspaces/"+env.wsSlug+"/collections/docs/items", map[string]interface{}{ + "title": "Readable", + }, env.ownerToken) + + // Viewer can list collections + rr := doRequestWithCookie(env.srv, "GET", "/api/v1/workspaces/"+env.wsSlug+"/collections", nil, env.viewerToken) + if rr.Code != http.StatusOK { + t.Errorf("expected 200 for viewer list collections, got %d", rr.Code) + } + + // Viewer can list items + rr = doRequestWithCookie(env.srv, "GET", "/api/v1/workspaces/"+env.wsSlug+"/collections/docs/items", nil, env.viewerToken) + if rr.Code != http.StatusOK { + t.Errorf("expected 200 for viewer list items, got %d", rr.Code) + } + + // Viewer can get workspace + rr = doRequestWithCookie(env.srv, "GET", "/api/v1/workspaces/"+env.wsSlug, nil, env.viewerToken) + if rr.Code != http.StatusOK { + t.Errorf("expected 200 for viewer get workspace, got %d", rr.Code) + } +} + +func TestRBAC_EditorBlockedFromAgentRoleMutations(t *testing.T) { + env := setupRBACEnv(t) + + rr := doRequestWithCookie(env.srv, "POST", "/api/v1/workspaces/"+env.wsSlug+"/agent-roles", map[string]interface{}{ + "name": "Test Role", + }, env.editorToken) + if rr.Code != http.StatusForbidden { + t.Errorf("expected 403 for editor create agent role, got %d: %s", rr.Code, rr.Body.String()) + } +} + +func TestRBAC_ViewerBlockedFromItemLinkMutations(t *testing.T) { + env := setupRBACEnv(t) + + // Create two items as owner + rr := doRequestWithCookie(env.srv, "POST", "/api/v1/workspaces/"+env.wsSlug+"/collections/docs/items", map[string]interface{}{ + "title": "Item A", + }, env.ownerToken) + if rr.Code != http.StatusCreated { + t.Fatalf("setup: create item A failed: %d %s", rr.Code, rr.Body.String()) + } + var itemA map[string]interface{} + parseJSON(t, rr, &itemA) + itemASlug := itemA["slug"].(string) + + rr = doRequestWithCookie(env.srv, "POST", "/api/v1/workspaces/"+env.wsSlug+"/collections/docs/items", map[string]interface{}{ + "title": "Item B", + }, env.ownerToken) + if rr.Code != http.StatusCreated { + t.Fatalf("setup: create item B failed: %d %s", rr.Code, rr.Body.String()) + } + var itemB map[string]interface{} + parseJSON(t, rr, &itemB) + itemBID := itemB["id"].(string) + + rr = doRequestWithCookie(env.srv, "POST", "/api/v1/workspaces/"+env.wsSlug+"/items/"+itemASlug+"/links", map[string]interface{}{ + "target_id": itemBID, + "link_type": "blocks", + }, env.viewerToken) + if rr.Code != http.StatusForbidden { + t.Errorf("expected 403 for viewer create item link, got %d: %s", rr.Code, rr.Body.String()) + } +} + +func TestRBAC_SearchScopedToUserWorkspaces(t *testing.T) { + env := setupRBACEnv(t) + + // Create an item in the workspace the editor belongs to + doRequestWithCookie(env.srv, "POST", "/api/v1/workspaces/"+env.wsSlug+"/collections/docs/items", map[string]interface{}{ + "title": "Visible Secret", + "content": "This should be found by the editor", + }, env.ownerToken) + + // Create a second workspace that the editor does NOT belong to + rr := doRequestWithCookie(env.srv, "POST", "/api/v1/workspaces", map[string]string{ + "name": "Private Workspace", + }, env.ownerToken) + if rr.Code != http.StatusCreated { + t.Fatalf("create private workspace: expected 201, got %d: %s", rr.Code, rr.Body.String()) + } + var privateWS map[string]interface{} + parseJSON(t, rr, &privateWS) + privateSlug := privateWS["slug"].(string) + + // Create an item in the private workspace + doRequestWithCookie(env.srv, "POST", "/api/v1/workspaces/"+privateSlug+"/collections/docs/items", map[string]interface{}{ + "title": "Hidden Secret", + "content": "This should NOT be found by the editor", + }, env.ownerToken) + + // Editor searches without workspace param — should only see their workspace's items + rr = doRequestWithCookie(env.srv, "GET", "/api/v1/search?q=Secret", nil, env.editorToken) + if rr.Code != http.StatusOK { + t.Fatalf("search: expected 200, got %d: %s", rr.Code, rr.Body.String()) + } + + var resp struct { + Results []map[string]interface{} `json:"results"` + Total int `json:"total"` + } + parseJSON(t, rr, &resp) + + // Should find only the visible item, not the one in the private workspace + if resp.Total != 1 { + t.Errorf("expected 1 result (only from editor's workspace), got %d", resp.Total) + for _, r := range resp.Results { + if item, ok := r["item"].(map[string]interface{}); ok { + t.Logf(" found: %v", item["title"]) + } + } + } + + // Owner searches without workspace param — should see items from both workspaces + rr = doRequestWithCookie(env.srv, "GET", "/api/v1/search?q=Secret", nil, env.ownerToken) + if rr.Code != http.StatusOK { + t.Fatalf("owner search: expected 200, got %d: %s", rr.Code, rr.Body.String()) + } + parseJSON(t, rr, &resp) + if resp.Total != 2 { + t.Errorf("expected 2 results for owner (both workspaces), got %d", resp.Total) + } +} diff --git a/internal/server/handlers_role_board.go b/internal/server/handlers_role_board.go index c82aff70..30eeebac 100644 --- a/internal/server/handlers_role_board.go +++ b/internal/server/handlers_role_board.go @@ -8,6 +8,9 @@ import ( // handleRoleBoardReorder updates role_sort_order for items within a lane. func (s *Server) handleRoleBoardReorder(w http.ResponseWriter, r *http.Request) { + if !requireMinRole(w, r, "editor") { + return + } workspaceID, ok := s.getWorkspaceID(w, r) if !ok { return @@ -20,7 +23,7 @@ func (s *Server) handleRoleBoardReorder(w http.ResponseWriter, r *http.Request) } if err := s.store.UpdateRoleSortOrder(workspaceID, updates); err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } @@ -29,6 +32,9 @@ func (s *Server) handleRoleBoardReorder(w http.ResponseWriter, r *http.Request) // handleRoleBoardLaneReorder updates sort_order for roles (lane ordering). func (s *Server) handleRoleBoardLaneReorder(w http.ResponseWriter, r *http.Request) { + if !requireMinRole(w, r, "editor") { + return + } workspaceID, ok := s.getWorkspaceID(w, r) if !ok { return @@ -41,7 +47,7 @@ func (s *Server) handleRoleBoardLaneReorder(w http.ResponseWriter, r *http.Reque } if err := s.store.UpdateAgentRoleOrder(workspaceID, updates); err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } @@ -62,7 +68,7 @@ func (s *Server) handleRoleBoard(w http.ResponseWriter, r *http.Request) { lanes, err := s.store.GetRoleBoardItems(workspaceID, params) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } diff --git a/internal/server/handlers_search.go b/internal/server/handlers_search.go index fe87ad3f..759d3edb 100644 --- a/internal/server/handlers_search.go +++ b/internal/server/handlers_search.go @@ -18,9 +18,34 @@ func (s *Server) handleSearch(w http.ResponseWriter, r *http.Request) { Workspace: r.URL.Query().Get("workspace"), } + // When no specific workspace is given, scope search to the user's + // workspaces so results never leak across workspace boundaries. + if params.Workspace == "" { + user := currentUser(r) + if user != nil { + workspaces, err := s.store.GetUserWorkspaces(user.ID) + if err != nil { + writeError(w, http.StatusInternalServerError, "internal_error", "Failed to resolve user workspaces") + return + } + for _, ws := range workspaces { + params.WorkspaceIDs = append(params.WorkspaceIDs, ws.ID) + } + // If user has no workspaces, return empty results + if len(params.WorkspaceIDs) == 0 { + writeJSON(w, http.StatusOK, map[string]interface{}{ + "results": []store.SearchResult{}, + "total": 0, + }) + return + } + } + // If no user (fresh install, no auth), allow unscoped search + } + results, err := s.store.Search(params) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if results == nil { diff --git a/internal/server/handlers_timeline.go b/internal/server/handlers_timeline.go index dfca5b46..540f51d2 100644 --- a/internal/server/handlers_timeline.go +++ b/internal/server/handlers_timeline.go @@ -55,7 +55,7 @@ func (s *Server) handleListItemTimeline(w http.ResponseWriter, r *http.Request) comments, err := s.store.ListCommentsBeforeTime(item.ID, before, beforeID, perSource) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } @@ -77,13 +77,13 @@ func (s *Server) handleListItemTimeline(w http.ResponseWriter, r *http.Request) activities, err := s.store.ListDocumentActivityBeforeTime(item.ID, before, beforeID, perSource) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } versions, err := s.store.ListItemVersionsBeforeTime(item.ID, before, beforeID, perSource) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } diff --git a/internal/server/handlers_tokens.go b/internal/server/handlers_tokens.go index 5d028d1b..2e3b38c0 100644 --- a/internal/server/handlers_tokens.go +++ b/internal/server/handlers_tokens.go @@ -33,7 +33,7 @@ func (s *Server) handleCreateToken(w http.ResponseWriter, r *http.Request) { userID := currentUserID(r) token, err := s.store.CreateAPIToken(userID, input) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } @@ -49,7 +49,7 @@ func (s *Server) handleListTokens(w http.ResponseWriter, r *http.Request) { tokens, err := s.store.ListAPITokens(workspaceID) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if tokens == nil { @@ -72,7 +72,7 @@ func (s *Server) handleDeleteToken(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusNotFound, "not_found", "Token not found") return } - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } @@ -91,7 +91,7 @@ func (s *Server) handleListUserTokens(w http.ResponseWriter, r *http.Request) { tokens, err := s.store.ListUserAPITokens(userID) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if tokens == nil { @@ -122,7 +122,7 @@ func (s *Server) handleCreateUserToken(w http.ResponseWriter, r *http.Request) { token, err := s.store.CreateAPIToken(userID, input) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } @@ -143,7 +143,7 @@ func (s *Server) handleDeleteUserToken(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusNotFound, "not_found", "Token not found") return } - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } diff --git a/internal/server/handlers_versions.go b/internal/server/handlers_versions.go index 658ca38b..93986289 100644 --- a/internal/server/handlers_versions.go +++ b/internal/server/handlers_versions.go @@ -19,7 +19,7 @@ func (s *Server) handleListVersions(w http.ResponseWriter, r *http.Request) { // Resolve diffs so API consumers always get full content versions, err := s.store.ListVersionsResolved(doc.ID, doc.Content) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if versions == nil { @@ -39,7 +39,7 @@ func (s *Server) handleGetVersion(w http.ResponseWriter, r *http.Request) { // Resolve diffs to return full content version, err := s.store.GetVersionResolved(versionID, doc.ID, doc.Content) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if version == nil { @@ -59,7 +59,7 @@ func (s *Server) handleGetDiff(w http.ResponseWriter, r *http.Request) { // Use resolved versions so diffs work correctly versions, err := s.store.ListVersionsResolved(doc.ID, doc.Content) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } diff --git a/internal/server/handlers_views.go b/internal/server/handlers_views.go index a1c73a03..c8fdabb8 100644 --- a/internal/server/handlers_views.go +++ b/internal/server/handlers_views.go @@ -19,7 +19,7 @@ func (s *Server) handleListViews(w http.ResponseWriter, r *http.Request) { collSlug := chi.URLParam(r, "collSlug") coll, err := s.store.GetCollectionBySlug(workspaceID, collSlug) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if coll == nil { @@ -29,7 +29,7 @@ func (s *Server) handleListViews(w http.ResponseWriter, r *http.Request) { views, err := s.store.ListViews(workspaceID, coll.ID) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if views == nil { @@ -41,6 +41,9 @@ func (s *Server) handleListViews(w http.ResponseWriter, r *http.Request) { // handleCreateView creates a new saved view for a collection. func (s *Server) handleCreateView(w http.ResponseWriter, r *http.Request) { + if !requireMinRole(w, r, "editor") { + return + } workspaceID, ok := s.getWorkspaceID(w, r) if !ok { return @@ -49,7 +52,7 @@ func (s *Server) handleCreateView(w http.ResponseWriter, r *http.Request) { collSlug := chi.URLParam(r, "collSlug") coll, err := s.store.GetCollectionBySlug(workspaceID, collSlug) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if coll == nil { @@ -72,7 +75,7 @@ func (s *Server) handleCreateView(w http.ResponseWriter, r *http.Request) { view, err := s.store.CreateView(workspaceID, input) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } @@ -81,6 +84,9 @@ func (s *Server) handleCreateView(w http.ResponseWriter, r *http.Request) { // handleUpdateView modifies an existing saved view. func (s *Server) handleUpdateView(w http.ResponseWriter, r *http.Request) { + if !requireMinRole(w, r, "editor") { + return + } _, ok := s.getWorkspaceID(w, r) if !ok { return @@ -100,7 +106,7 @@ func (s *Server) handleUpdateView(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusNotFound, "not_found", "View not found") return } - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } @@ -109,6 +115,9 @@ func (s *Server) handleUpdateView(w http.ResponseWriter, r *http.Request) { // handleDeleteView removes a saved view. func (s *Server) handleDeleteView(w http.ResponseWriter, r *http.Request) { + if !requireMinRole(w, r, "editor") { + return + } _, ok := s.getWorkspaceID(w, r) if !ok { return @@ -121,7 +130,7 @@ func (s *Server) handleDeleteView(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusNotFound, "not_found", "View not found") return } - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } diff --git a/internal/server/handlers_webhooks.go b/internal/server/handlers_webhooks.go index 2f781db1..13568e8f 100644 --- a/internal/server/handlers_webhooks.go +++ b/internal/server/handlers_webhooks.go @@ -8,6 +8,7 @@ import ( "github.com/go-chi/chi/v5" "github.com/xarmian/pad/internal/models" + "github.com/xarmian/pad/internal/webhooks" ) // dispatchWebhook fires a webhook event if a dispatcher is configured. @@ -20,6 +21,9 @@ func (s *Server) dispatchWebhook(workspaceID, event string, data interface{}) { // handleCreateWebhook registers a new webhook for a workspace. func (s *Server) handleCreateWebhook(w http.ResponseWriter, r *http.Request) { + if !requireMinRole(w, r, "owner") { + return + } workspaceID, ok := s.getWorkspaceID(w, r) if !ok { return @@ -36,9 +40,15 @@ func (s *Server) handleCreateWebhook(w http.ResponseWriter, r *http.Request) { return } + // Validate URL to prevent SSRF attacks + if err := webhooks.ValidateWebhookURL(input.URL); err != nil { + writeError(w, http.StatusBadRequest, "bad_request", "Invalid webhook URL: "+err.Error()) + return + } + hook, err := s.store.CreateWebhook(workspaceID, input) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } @@ -54,7 +64,7 @@ func (s *Server) handleListWebhooks(w http.ResponseWriter, r *http.Request) { hooks, err := s.store.ListWebhooks(workspaceID) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if hooks == nil { @@ -66,6 +76,9 @@ func (s *Server) handleListWebhooks(w http.ResponseWriter, r *http.Request) { // handleDeleteWebhook removes a webhook by ID. func (s *Server) handleDeleteWebhook(w http.ResponseWriter, r *http.Request) { + if !requireMinRole(w, r, "owner") { + return + } _, ok := s.getWorkspaceID(w, r) if !ok { return @@ -77,7 +90,7 @@ func (s *Server) handleDeleteWebhook(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusNotFound, "not_found", "Webhook not found") return } - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } @@ -86,6 +99,9 @@ func (s *Server) handleDeleteWebhook(w http.ResponseWriter, r *http.Request) { // handleTestWebhook sends a test payload to the specified webhook. func (s *Server) handleTestWebhook(w http.ResponseWriter, r *http.Request) { + if !requireMinRole(w, r, "owner") { + return + } _, ok := s.getWorkspaceID(w, r) if !ok { return @@ -94,7 +110,7 @@ func (s *Server) handleTestWebhook(w http.ResponseWriter, r *http.Request) { webhookID := chi.URLParam(r, "webhookID") hook, err := s.store.GetWebhook(webhookID) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if hook == nil { diff --git a/internal/server/handlers_workspaces.go b/internal/server/handlers_workspaces.go index 9211da0f..92b6624a 100644 --- a/internal/server/handlers_workspaces.go +++ b/internal/server/handlers_workspaces.go @@ -101,7 +101,7 @@ func (s *Server) handleListWorkspaces(w http.ResponseWriter, r *http.Request) { if user != nil && user.Role != "admin" { workspaces, err := s.store.GetUserWorkspaces(user.ID) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if workspaces == nil { @@ -114,7 +114,7 @@ func (s *Server) handleListWorkspaces(w http.ResponseWriter, r *http.Request) { // Admin users (or fresh-install with no users) see all workspaces. workspaces, err := s.store.ListWorkspaces() if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if workspaces == nil { @@ -141,7 +141,7 @@ func (s *Server) handleCreateWorkspace(w http.ResponseWriter, r *http.Request) { ws, err := s.store.CreateWorkspace(input) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } @@ -163,7 +163,7 @@ func (s *Server) handleGetWorkspace(w http.ResponseWriter, r *http.Request) { slug := chi.URLParam(r, "slug") ws, err := s.store.GetWorkspaceBySlug(slug) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if ws == nil { @@ -174,6 +174,9 @@ func (s *Server) handleGetWorkspace(w http.ResponseWriter, r *http.Request) { } func (s *Server) handleUpdateWorkspace(w http.ResponseWriter, r *http.Request) { + if !requireMinRole(w, r, "owner") { + return + } slug := chi.URLParam(r, "slug") var input models.WorkspaceUpdate @@ -188,7 +191,7 @@ func (s *Server) handleUpdateWorkspace(w http.ResponseWriter, r *http.Request) { ws, err := s.store.UpdateWorkspace(slug, input) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return } if ws == nil { @@ -202,6 +205,9 @@ func (s *Server) handleUpdateWorkspace(w http.ResponseWriter, r *http.Request) { } func (s *Server) handleDeleteWorkspace(w http.ResponseWriter, r *http.Request) { + if !requireMinRole(w, r, "owner") { + return + } slug := chi.URLParam(r, "slug") err := s.store.DeleteWorkspace(slug) if err != nil { @@ -212,6 +218,9 @@ func (s *Server) handleDeleteWorkspace(w http.ResponseWriter, r *http.Request) { } func (s *Server) handleExportWorkspace(w http.ResponseWriter, r *http.Request) { + if !requireMinRole(w, r, "owner") { + return + } slug := chi.URLParam(r, "slug") export, err := s.store.ExportWorkspace(slug) if err != nil { diff --git a/internal/server/middleware_auth.go b/internal/server/middleware_auth.go index 0aef1287..fcd7dc23 100644 --- a/internal/server/middleware_auth.go +++ b/internal/server/middleware_auth.go @@ -283,6 +283,16 @@ func requireRole(r *http.Request, minRole string) bool { return roleLevel(role) >= roleLevel(minRole) } +// requireMinRole checks role and writes a 403 if insufficient. +// Returns true if the request should continue, false if it was rejected. +func requireMinRole(w http.ResponseWriter, r *http.Request, minRole string) bool { + if requireRole(r, minRole) { + return true + } + writeError(w, http.StatusForbidden, "forbidden", "Insufficient permissions") + return false +} + // roleLevel returns a numeric level for role comparison. // Higher values indicate more permissions. func roleLevel(role string) int { diff --git a/internal/server/middleware_csrf.go b/internal/server/middleware_csrf.go new file mode 100644 index 00000000..b81d2d97 --- /dev/null +++ b/internal/server/middleware_csrf.go @@ -0,0 +1,117 @@ +package server + +import ( + "crypto/rand" + "encoding/hex" + "net/http" + "strings" +) + +const ( + csrfCookie = "pad_csrf" + csrfHeader = "X-CSRF-Token" + csrfTokenLen = 32 // 32 bytes = 64 hex chars +) + +// CSRFProtect implements the double-submit cookie pattern for CSRF protection. +// It validates that state-changing requests (POST, PATCH, PUT, DELETE) from +// cookie-authenticated sessions include a matching CSRF token in both the +// cookie and the X-CSRF-Token header. +// +// Requests authenticated via Bearer tokens (API tokens / CLI) are exempt +// because they are not vulnerable to CSRF attacks — the browser never +// attaches Authorization headers automatically. +// +// Safe methods (GET, HEAD, OPTIONS) are always allowed through. +func (s *Server) CSRFProtect(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // Safe methods are exempt + switch r.Method { + case http.MethodGet, http.MethodHead, http.MethodOptions: + next.ServeHTTP(w, r) + return + } + + // Non-API paths are exempt (SPA static files, etc.) + if !strings.HasPrefix(r.URL.Path, "/api/") { + next.ServeHTTP(w, r) + return + } + + // Auth endpoints that need to work before a CSRF token exists + // (login, register, bootstrap, password reset) + if strings.HasPrefix(r.URL.Path, "/api/v1/auth/") { + next.ServeHTTP(w, r) + return + } + + // Bearer token requests are not vulnerable to CSRF — skip + if auth := r.Header.Get("Authorization"); strings.HasPrefix(auth, "Bearer ") { + next.ServeHTTP(w, r) + return + } + + // No users exist (fresh install) — skip CSRF + count, err := s.store.UserCount() + if err != nil || count == 0 { + next.ServeHTTP(w, r) + return + } + + // Cookie-based session: require CSRF token + cookie, err := r.Cookie(csrfCookie) + if err != nil || cookie.Value == "" { + writeError(w, http.StatusForbidden, "csrf_error", "Missing CSRF token") + return + } + + headerToken := r.Header.Get(csrfHeader) + if headerToken == "" { + writeError(w, http.StatusForbidden, "csrf_error", "Missing CSRF header") + return + } + + if cookie.Value != headerToken { + writeError(w, http.StatusForbidden, "csrf_error", "CSRF token mismatch") + return + } + + next.ServeHTTP(w, r) + }) +} + +// setCSRFCookie writes a new CSRF token cookie. The cookie is NOT HttpOnly +// so that JavaScript can read it and send it back as a header. +func setCSRFCookie(w http.ResponseWriter, ttl int, secure bool) { + token := generateCSRFToken() + http.SetCookie(w, &http.Cookie{ + Name: csrfCookie, + Value: token, + Path: "/", + MaxAge: ttl, + HttpOnly: false, // Must be readable by JS + Secure: secure, + SameSite: http.SameSiteLaxMode, + }) +} + +// clearCSRFCookie removes the CSRF cookie (e.g. on logout). +func clearCSRFCookie(w http.ResponseWriter) { + http.SetCookie(w, &http.Cookie{ + Name: csrfCookie, + Value: "", + Path: "/", + MaxAge: -1, + HttpOnly: false, + SameSite: http.SameSiteLaxMode, + }) +} + +// generateCSRFToken returns a cryptographically random hex string. +func generateCSRFToken() string { + b := make([]byte, csrfTokenLen) + if _, err := rand.Read(b); err != nil { + panic("csrf: failed to generate random token: " + err.Error()) + } + return hex.EncodeToString(b) +} diff --git a/internal/server/middleware_csrf_test.go b/internal/server/middleware_csrf_test.go new file mode 100644 index 00000000..8d04e2a7 --- /dev/null +++ b/internal/server/middleware_csrf_test.go @@ -0,0 +1,249 @@ +package server + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +func TestCSRF_SafeMethodsAllowed(t *testing.T) { + srv := testServer(t) + + for _, method := range []string{http.MethodGet, http.MethodHead, http.MethodOptions} { + req := httptest.NewRequest(method, "/api/v1/workspaces", nil) + w := httptest.NewRecorder() + srv.ServeHTTP(w, req) + + if w.Code == http.StatusForbidden { + body := w.Body.String() + if strings.Contains(body, "csrf") || strings.Contains(body, "CSRF") { + t.Errorf("%s should not be blocked by CSRF, got 403 with body: %s", method, body) + } + } + } +} + +func TestCSRF_BearerTokenExempt(t *testing.T) { + srv := testServer(t) + + // Bootstrap admin so auth is required + token := bootstrapFirstUser(t, srv, "admin@test.com", "Admin") + + // POST with Bearer token (session token), no CSRF — should NOT get CSRF error + req := httptest.NewRequest(http.MethodPost, "/api/v1/workspaces", + strings.NewReader(`{"name":"test"}`)) + req.Header.Set("Authorization", "Bearer "+token) + req.Header.Set("Content-Type", "application/json") + req.RemoteAddr = "192.0.2.1:1234" + w := httptest.NewRecorder() + srv.ServeHTTP(w, req) + + if w.Code == http.StatusForbidden { + body := w.Body.String() + if strings.Contains(body, "csrf") || strings.Contains(body, "CSRF") { + t.Errorf("Bearer token request should be CSRF-exempt, got 403: %s", body) + } + } +} + +func TestCSRF_AuthEndpointsExempt(t *testing.T) { + srv := testServer(t) + + // Auth endpoints should work without CSRF token + endpoints := []string{ + "/api/v1/auth/login", + "/api/v1/auth/register", + "/api/v1/auth/logout", + "/api/v1/auth/forgot-password", + "/api/v1/auth/reset-password", + } + + for _, ep := range endpoints { + req := httptest.NewRequest(http.MethodPost, ep, strings.NewReader(`{}`)) + req.Header.Set("Content-Type", "application/json") + req.RemoteAddr = "192.0.2.1:1234" + w := httptest.NewRecorder() + srv.ServeHTTP(w, req) + + if w.Code == http.StatusForbidden { + body := w.Body.String() + if strings.Contains(body, "csrf") || strings.Contains(body, "CSRF") { + t.Errorf("%s should be CSRF-exempt, got 403: %s", ep, body) + } + } + } +} + +func TestCSRF_MissingTokenBlocked(t *testing.T) { + srv := testServer(t) + + // Bootstrap and login to get session token + bootstrapFirstUser(t, srv, "admin@test.com", "Admin") + sessionToken := loginUser(t, srv, "admin@test.com", "password123") + + // POST with session cookie but no CSRF token at all + req := httptest.NewRequest(http.MethodPost, "/api/v1/workspaces", + strings.NewReader(`{"name":"test"}`)) + req.Header.Set("Content-Type", "application/json") + req.RemoteAddr = "192.0.2.1:1234" + req.AddCookie(&http.Cookie{Name: "pad_session", Value: sessionToken}) + w := httptest.NewRecorder() + srv.ServeHTTP(w, req) + + if w.Code != http.StatusForbidden { + t.Errorf("expected 403 for missing CSRF token, got %d: %s", w.Code, w.Body.String()) + } +} + +func TestCSRF_MismatchBlocked(t *testing.T) { + srv := testServer(t) + + bootstrapFirstUser(t, srv, "admin@test.com", "Admin") + sessionToken := loginUser(t, srv, "admin@test.com", "password123") + + // POST with session cookie + CSRF cookie but WRONG header value + req := httptest.NewRequest(http.MethodPost, "/api/v1/workspaces", + strings.NewReader(`{"name":"test"}`)) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("X-CSRF-Token", "wrong-token") + req.RemoteAddr = "192.0.2.1:1234" + req.AddCookie(&http.Cookie{Name: "pad_session", Value: sessionToken}) + req.AddCookie(&http.Cookie{Name: "pad_csrf", Value: "correct-token"}) + w := httptest.NewRecorder() + srv.ServeHTTP(w, req) + + if w.Code != http.StatusForbidden { + t.Errorf("expected 403 for CSRF mismatch, got %d: %s", w.Code, w.Body.String()) + } +} + +func TestCSRF_MatchingTokenAllowed(t *testing.T) { + srv := testServer(t) + + bootstrapFirstUser(t, srv, "admin@test.com", "Admin") + sessionToken := loginUser(t, srv, "admin@test.com", "password123") + + // POST with matching CSRF cookie + header + csrfVal := "matching-csrf-token" + req := httptest.NewRequest(http.MethodPost, "/api/v1/workspaces", + strings.NewReader(`{"name":"csrftest"}`)) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("X-CSRF-Token", csrfVal) + req.RemoteAddr = "192.0.2.1:1234" + req.AddCookie(&http.Cookie{Name: "pad_session", Value: sessionToken}) + req.AddCookie(&http.Cookie{Name: "pad_csrf", Value: csrfVal}) + w := httptest.NewRecorder() + srv.ServeHTTP(w, req) + + // Should NOT be blocked by CSRF + if w.Code == http.StatusForbidden { + body := w.Body.String() + if strings.Contains(body, "csrf") || strings.Contains(body, "CSRF") { + t.Errorf("matching CSRF token should be allowed, got 403: %s", body) + } + } + // Workspace create should succeed (201) + if w.Code != http.StatusCreated { + t.Errorf("expected 201 for workspace create with valid CSRF, got %d: %s", w.Code, w.Body.String()) + } +} + +func TestCSRF_FreshInstallExempt(t *testing.T) { + srv := testServer(t) + + // No users → fresh install → CSRF should be skipped + req := httptest.NewRequest(http.MethodPost, "/api/v1/workspaces", + strings.NewReader(`{"name":"test"}`)) + req.Header.Set("Content-Type", "application/json") + req.RemoteAddr = "192.0.2.1:1234" + w := httptest.NewRecorder() + srv.ServeHTTP(w, req) + + if w.Code == http.StatusForbidden { + body := w.Body.String() + if strings.Contains(body, "csrf") || strings.Contains(body, "CSRF") { + t.Errorf("fresh install should be CSRF-exempt, got 403: %s", body) + } + } +} + +func TestCSRF_LoginSetsCSRFCookie(t *testing.T) { + srv := testServer(t) + + bootstrapFirstUser(t, srv, "admin@test.com", "Admin") + + // Login + req := httptest.NewRequest(http.MethodPost, "/api/v1/auth/login", + strings.NewReader(`{"email":"admin@test.com","password":"password123"}`)) + req.Header.Set("Content-Type", "application/json") + req.RemoteAddr = "192.0.2.1:1234" + w := httptest.NewRecorder() + srv.ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("login failed: %d %s", w.Code, w.Body.String()) + } + + // Check that CSRF cookie is set + var foundCSRF bool + for _, c := range w.Result().Cookies() { + if c.Name == "pad_csrf" { + foundCSRF = true + if c.HttpOnly { + t.Error("CSRF cookie must not be HttpOnly") + } + if c.Value == "" { + t.Error("CSRF cookie value must not be empty") + } + } + } + if !foundCSRF { + t.Error("login response should set pad_csrf cookie") + } +} + +func TestCSRF_LogoutClearsCSRFCookie(t *testing.T) { + srv := testServer(t) + + bootstrapFirstUser(t, srv, "admin@test.com", "Admin") + sessionToken := loginUser(t, srv, "admin@test.com", "password123") + + // Logout (auth endpoints are CSRF-exempt) + req := httptest.NewRequest(http.MethodPost, "/api/v1/auth/logout", nil) + req.AddCookie(&http.Cookie{Name: "pad_session", Value: sessionToken}) + req.RemoteAddr = "192.0.2.1:1234" + w := httptest.NewRecorder() + srv.ServeHTTP(w, req) + + // Check CSRF cookie is cleared + for _, c := range w.Result().Cookies() { + if c.Name == "pad_csrf" { + if c.MaxAge >= 0 { + t.Errorf("expected CSRF cookie to be cleared (MaxAge < 0), got MaxAge=%d", c.MaxAge) + } + } + } +} + +func TestCSRF_AllMutationMethodsBlocked(t *testing.T) { + srv := testServer(t) + + bootstrapFirstUser(t, srv, "admin@test.com", "Admin") + sessionToken := loginUser(t, srv, "admin@test.com", "password123") + + // All state-changing methods should be blocked without CSRF + for _, method := range []string{http.MethodPost, http.MethodPatch, http.MethodPut, http.MethodDelete} { + req := httptest.NewRequest(method, "/api/v1/workspaces", + strings.NewReader(`{"name":"test"}`)) + req.Header.Set("Content-Type", "application/json") + req.RemoteAddr = "192.0.2.1:1234" + req.AddCookie(&http.Cookie{Name: "pad_session", Value: sessionToken}) + w := httptest.NewRecorder() + srv.ServeHTTP(w, req) + + if w.Code != http.StatusForbidden { + t.Errorf("%s without CSRF should be 403, got %d", method, w.Code) + } + } +} diff --git a/internal/server/middleware_ratelimit.go b/internal/server/middleware_ratelimit.go new file mode 100644 index 00000000..dd59267e --- /dev/null +++ b/internal/server/middleware_ratelimit.go @@ -0,0 +1,205 @@ +package server + +import ( + "net/http" + "strconv" + "strings" + "sync" + "time" + + "golang.org/x/time/rate" +) + +// rateLimitConfig holds the rate and burst for a limiter. +type rateLimitConfig struct { + Rate rate.Limit // events per second + Burst int // max burst +} + +// ipRateLimiter tracks per-key rate limiters with automatic cleanup. +type ipRateLimiter struct { + mu sync.Mutex + limiters map[string]*rateLimiterEntry + config rateLimitConfig +} + +type rateLimiterEntry struct { + limiter *rate.Limiter + lastSeen time.Time +} + +func newIPRateLimiter(cfg rateLimitConfig) *ipRateLimiter { + rl := &ipRateLimiter{ + limiters: make(map[string]*rateLimiterEntry), + config: cfg, + } + // Background cleanup of stale entries every 5 minutes + go rl.cleanup() + return rl +} + +func (rl *ipRateLimiter) getLimiter(key string) *rate.Limiter { + rl.mu.Lock() + defer rl.mu.Unlock() + + entry, exists := rl.limiters[key] + if !exists { + limiter := rate.NewLimiter(rl.config.Rate, rl.config.Burst) + rl.limiters[key] = &rateLimiterEntry{ + limiter: limiter, + lastSeen: time.Now(), + } + return limiter + } + entry.lastSeen = time.Now() + return entry.limiter +} + +func (rl *ipRateLimiter) cleanup() { + for { + time.Sleep(5 * time.Minute) + rl.mu.Lock() + for key, entry := range rl.limiters { + if time.Since(entry.lastSeen) > 30*time.Minute { + delete(rl.limiters, key) + } + } + rl.mu.Unlock() + } +} + +// RateLimiters holds all the rate limiters used by the server. +type RateLimiters struct { + // Auth endpoints: strict limits per IP + Auth *ipRateLimiter + // Password reset: per-IP + PasswordReset *ipRateLimiter + // Registration: per-IP + Register *ipRateLimiter + // API: per-user (authenticated) + API *ipRateLimiter + // Search: per-user or per-IP + Search *ipRateLimiter +} + +// NewRateLimiters creates rate limiters with sensible defaults. +func NewRateLimiters() *RateLimiters { + return &RateLimiters{ + // Login: 5 attempts per minute per IP (= 5/60 per second, burst 5) + Auth: newIPRateLimiter(rateLimitConfig{ + Rate: rate.Limit(5.0 / 60.0), + Burst: 5, + }), + // Password reset: 3 per hour per IP (= 3/3600 per second, burst 3) + PasswordReset: newIPRateLimiter(rateLimitConfig{ + Rate: rate.Limit(3.0 / 3600.0), + Burst: 3, + }), + // Registration: 5 per hour per IP (= 5/3600 per second, burst 5) + Register: newIPRateLimiter(rateLimitConfig{ + Rate: rate.Limit(5.0 / 3600.0), + Burst: 5, + }), + // API: 100 requests per minute per user/IP (= 100/60 per second, burst 20) + API: newIPRateLimiter(rateLimitConfig{ + Rate: rate.Limit(100.0 / 60.0), + Burst: 20, + }), + // Search: 30 requests per minute per user/IP (= 30/60 per second, burst 10) + Search: newIPRateLimiter(rateLimitConfig{ + Rate: rate.Limit(30.0 / 60.0), + Burst: 10, + }), + } +} + +// RateLimit is the general-purpose rate limiting middleware. +// It applies different limits based on the endpoint being hit. +func (s *Server) RateLimit(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if s.rateLimiters == nil { + next.ServeHTTP(w, r) + return + } + + path := r.URL.Path + + // Only rate-limit API endpoints + if !strings.HasPrefix(path, "/api/") { + next.ServeHTTP(w, r) + return + } + + ip := clientIP(r) + + // Auth-specific rate limits + if strings.HasPrefix(path, "/api/v1/auth/") { + var limiter *ipRateLimiter + switch { + case path == "/api/v1/auth/login" || path == "/api/v1/auth/bootstrap": + limiter = s.rateLimiters.Auth + case path == "/api/v1/auth/forgot-password" || path == "/api/v1/auth/reset-password": + limiter = s.rateLimiters.PasswordReset + case path == "/api/v1/auth/register": + limiter = s.rateLimiters.Register + default: + // Other auth endpoints (session check, logout) — use general API limit + limiter = s.rateLimiters.API + } + + if limiter != nil && !limiter.getLimiter(ip).Allow() { + writeTooManyRequests(w) + return + } + next.ServeHTTP(w, r) + return + } + + // Search endpoint + if path == "/api/v1/search" { + key := rateLimitKey(r, ip) + if !s.rateLimiters.Search.getLimiter(key).Allow() { + writeTooManyRequests(w) + return + } + next.ServeHTTP(w, r) + return + } + + // General API rate limit + key := rateLimitKey(r, ip) + if !s.rateLimiters.API.getLimiter(key).Allow() { + writeTooManyRequests(w) + return + } + + next.ServeHTTP(w, r) + }) +} + +// rateLimitKey returns a key for rate limiting: user ID if authenticated, IP otherwise. +func rateLimitKey(r *http.Request, ip string) string { + if user := currentUser(r); user != nil { + return "user:" + user.ID + } + return "ip:" + ip +} + +// clientIP extracts the client IP from RemoteAddr. This is safe because +// chimiddleware.RealIP runs earlier in the chain and overwrites RemoteAddr +// with the trusted value from X-Real-IP / X-Forwarded-For. We deliberately +// do NOT read proxy headers here to prevent clients from spoofing their IP +// to bypass rate limits. +func clientIP(r *http.Request) string { + host := r.RemoteAddr + if idx := strings.LastIndex(host, ":"); idx != -1 { + return host[:idx] + } + return host +} + +// writeTooManyRequests sends a 429 response with a Retry-After header. +func writeTooManyRequests(w http.ResponseWriter) { + w.Header().Set("Retry-After", strconv.Itoa(60)) // suggest retry after 60s + writeError(w, http.StatusTooManyRequests, "rate_limited", "Too many requests. Please try again later.") +} diff --git a/internal/server/middleware_ratelimit_test.go b/internal/server/middleware_ratelimit_test.go new file mode 100644 index 00000000..b77aca6a --- /dev/null +++ b/internal/server/middleware_ratelimit_test.go @@ -0,0 +1,148 @@ +package server + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +func TestRateLimit_AuthEndpointLimited(t *testing.T) { + srv := testServer(t) + + // Bootstrap so auth endpoints actually process (not just "setup required") + bootstrapFirstUser(t, srv, "admin@test.com", "Admin") + + // Login attempts should be rate-limited after burst (5) + for i := 0; i < 5; i++ { + req := httptest.NewRequest(http.MethodPost, "/api/v1/auth/login", + strings.NewReader(`{"email":"wrong@test.com","password":"wrong"}`)) + req.Header.Set("Content-Type", "application/json") + req.RemoteAddr = "10.0.0.1:1234" + w := httptest.NewRecorder() + srv.ServeHTTP(w, req) + // These should go through (even if returning 401) + if w.Code == http.StatusTooManyRequests { + t.Fatalf("request %d should not be rate-limited yet", i+1) + } + } + + // The 6th should be rate-limited + req := httptest.NewRequest(http.MethodPost, "/api/v1/auth/login", + strings.NewReader(`{"email":"wrong@test.com","password":"wrong"}`)) + req.Header.Set("Content-Type", "application/json") + req.RemoteAddr = "10.0.0.1:1234" + w := httptest.NewRecorder() + srv.ServeHTTP(w, req) + + if w.Code != http.StatusTooManyRequests { + t.Errorf("expected 429 after burst, got %d", w.Code) + } + + // Check Retry-After header + if w.Header().Get("Retry-After") == "" { + t.Error("expected Retry-After header on 429 response") + } +} + +func TestRateLimit_DifferentIPsNotAffected(t *testing.T) { + srv := testServer(t) + bootstrapFirstUser(t, srv, "admin@test.com", "Admin") + + // Exhaust rate limit for IP 10.0.0.1 + for i := 0; i < 6; i++ { + req := httptest.NewRequest(http.MethodPost, "/api/v1/auth/login", + strings.NewReader(`{"email":"wrong@test.com","password":"wrong"}`)) + req.Header.Set("Content-Type", "application/json") + req.RemoteAddr = "10.0.0.1:1234" + w := httptest.NewRecorder() + srv.ServeHTTP(w, req) + } + + // Different IP should still be allowed + req := httptest.NewRequest(http.MethodPost, "/api/v1/auth/login", + strings.NewReader(`{"email":"wrong@test.com","password":"wrong"}`)) + req.Header.Set("Content-Type", "application/json") + req.RemoteAddr = "10.0.0.2:1234" + w := httptest.NewRecorder() + srv.ServeHTTP(w, req) + + if w.Code == http.StatusTooManyRequests { + t.Error("different IP should not be rate-limited") + } +} + +func TestRateLimit_SearchEndpointLimited(t *testing.T) { + srv := testServer(t) + + // Search limiter has burst=10, so first 10 should succeed + for i := 0; i < 10; i++ { + req := httptest.NewRequest(http.MethodGet, "/api/v1/search?q=test", nil) + req.RemoteAddr = "10.0.0.3:1234" + w := httptest.NewRecorder() + srv.ServeHTTP(w, req) + if w.Code == http.StatusTooManyRequests { + t.Fatalf("request %d should not be rate-limited yet (search burst=10)", i+1) + } + } + + // The 11th should be rate-limited + req := httptest.NewRequest(http.MethodGet, "/api/v1/search?q=test", nil) + req.RemoteAddr = "10.0.0.3:1234" + w := httptest.NewRecorder() + srv.ServeHTTP(w, req) + if w.Code != http.StatusTooManyRequests { + t.Errorf("expected 429 after search burst, got %d", w.Code) + } +} + +func TestRateLimit_NonAPIPathsExempt(t *testing.T) { + srv := testServer(t) + + // Non-API paths should not be rate-limited + for i := 0; i < 50; i++ { + req := httptest.NewRequest(http.MethodGet, "/login", nil) + req.RemoteAddr = "10.0.0.4:1234" + w := httptest.NewRecorder() + srv.ServeHTTP(w, req) + if w.Code == http.StatusTooManyRequests { + t.Fatalf("non-API request %d should not be rate-limited", i+1) + } + } +} + +func TestClientIP(t *testing.T) { + // clientIP only reads RemoteAddr (proxy headers are handled by chimiddleware.RealIP) + tests := []struct { + name string + remoteAddr string + want string + }{ + {"with port", "192.168.1.1:1234", "192.168.1.1"}, + {"no port", "10.0.0.1", "10.0.0.1"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + req := httptest.NewRequest("GET", "/", nil) + req.RemoteAddr = tt.remoteAddr + got := clientIP(req) + if got != tt.want { + t.Errorf("clientIP() = %q, want %q", got, tt.want) + } + }) + } +} + +func TestClientIP_IgnoresProxyHeaders(t *testing.T) { + // Ensure clientIP does NOT trust X-Real-IP or X-Forwarded-For + req := httptest.NewRequest("GET", "/", nil) + req.RemoteAddr = "192.168.1.1:1234" + req.Header.Set("X-Real-IP", "10.0.0.99") + req.Header.Set("X-Forwarded-For", "10.0.0.88") + + got := clientIP(req) + if got != "192.168.1.1" { + t.Errorf("clientIP should ignore proxy headers, got %q", got) + } +} diff --git a/internal/server/middleware_security.go b/internal/server/middleware_security.go new file mode 100644 index 00000000..941e6c55 --- /dev/null +++ b/internal/server/middleware_security.go @@ -0,0 +1,62 @@ +package server + +import ( + "net/http" + "strings" +) + +// SecurityHeaders adds standard security headers to all responses. +// These protect against common web vulnerabilities like XSS, clickjacking, +// and MIME type sniffing. +func SecurityHeaders(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + h := w.Header() + + // Prevent the browser from MIME-sniffing the content type + h.Set("X-Content-Type-Options", "nosniff") + + // Prevent the page from being embedded in frames (clickjacking protection) + h.Set("X-Frame-Options", "DENY") + + // Control referrer information sent with requests + h.Set("Referrer-Policy", "strict-origin-when-cross-origin") + + // Restrict browser features the app doesn't need + h.Set("Permissions-Policy", "camera=(), microphone=(), geolocation=()") + + // CSP: allow self-sourced scripts/styles, plus inline styles for Svelte + h.Set("Content-Security-Policy", + "default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; font-src 'self'; connect-src 'self'; frame-ancestors 'none'") + + next.ServeHTTP(w, r) + }) +} + +// StrictTransportSecurity adds HSTS header when secure cookies are enabled +// (indicating the server is behind TLS). +func StrictTransportSecurity(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Strict-Transport-Security", "max-age=31536000; includeSubDomains") + next.ServeHTTP(w, r) + }) +} + +// parseCORSOrigins parses a comma-separated list of origins into a slice. +// Returns default localhost origins if the input is empty. +func parseCORSOrigins(origins string) []string { + if origins == "" { + return []string{"http://localhost:*", "http://127.0.0.1:*"} + } + + var result []string + for _, origin := range strings.Split(origins, ",") { + origin = strings.TrimSpace(origin) + if origin != "" { + result = append(result, origin) + } + } + if len(result) == 0 { + return []string{"http://localhost:*", "http://127.0.0.1:*"} + } + return result +} diff --git a/internal/server/middleware_security_test.go b/internal/server/middleware_security_test.go new file mode 100644 index 00000000..a2f71a2b --- /dev/null +++ b/internal/server/middleware_security_test.go @@ -0,0 +1,66 @@ +package server + +import ( + "net/http" + "net/http/httptest" + "testing" +) + +func TestSecurityHeaders(t *testing.T) { + srv := testServer(t) + + req := httptest.NewRequest(http.MethodGet, "/api/v1/health", nil) + req.RemoteAddr = "192.0.2.1:1234" + w := httptest.NewRecorder() + srv.ServeHTTP(w, req) + + headers := map[string]string{ + "X-Content-Type-Options": "nosniff", + "X-Frame-Options": "DENY", + "Referrer-Policy": "strict-origin-when-cross-origin", + "Permissions-Policy": "camera=(), microphone=(), geolocation=()", + } + + for name, expected := range headers { + got := w.Header().Get(name) + if got != expected { + t.Errorf("%s = %q, want %q", name, got, expected) + } + } + + // CSP should be set + csp := w.Header().Get("Content-Security-Policy") + if csp == "" { + t.Error("Content-Security-Policy header not set") + } + + // HSTS should NOT be set when secureCookies is false (default) + if hsts := w.Header().Get("Strict-Transport-Security"); hsts != "" { + t.Errorf("HSTS should not be set when secureCookies is off, got %q", hsts) + } +} + +func TestParseCORSOrigins(t *testing.T) { + tests := []struct { + input string + want []string + }{ + {"", []string{"http://localhost:*", "http://127.0.0.1:*"}}, + {"https://app.pad.dev", []string{"https://app.pad.dev"}}, + {"https://app.pad.dev, https://admin.pad.dev", []string{"https://app.pad.dev", "https://admin.pad.dev"}}, + {" , ", []string{"http://localhost:*", "http://127.0.0.1:*"}}, // empty after trim + } + + for _, tt := range tests { + got := parseCORSOrigins(tt.input) + if len(got) != len(tt.want) { + t.Errorf("parseCORSOrigins(%q) = %v, want %v", tt.input, got, tt.want) + continue + } + for i := range got { + if got[i] != tt.want[i] { + t.Errorf("parseCORSOrigins(%q)[%d] = %q, want %q", tt.input, i, got[i], tt.want[i]) + } + } + } +} diff --git a/internal/server/server.go b/internal/server/server.go index 581236ce..71cbb503 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -7,6 +7,7 @@ import ( "log" "net/http" "strings" + "sync" "github.com/go-chi/chi/v5" chimiddleware "github.com/go-chi/chi/v5/middleware" @@ -20,22 +21,27 @@ import ( ) type Server struct { - store *store.Store - router *chi.Mux - webFS fs.FS // embedded web UI static files (optional) - events *events.Bus // real-time event bus (optional) - webhooks *webhooks.Dispatcher // webhook dispatcher (optional) - email *email.Sender // transactional email sender (optional) - baseURL string // public base URL for generating links (e.g. invite URLs) - version string // release version (e.g. "dev", "1.2.3") - commit string // git commit hash - buildTime string // build timestamp + store *store.Store + router *chi.Mux + routerOnce sync.Once // ensures setupRouter runs once, after all config + webFS fs.FS // embedded web UI static files (optional) + events *events.Bus // real-time event bus (optional) + webhooks *webhooks.Dispatcher // webhook dispatcher (optional) + email *email.Sender // transactional email sender (optional) + rateLimiters *RateLimiters // per-endpoint rate limiters + baseURL string // public base URL for generating links (e.g. invite URLs) + corsOrigins string // comma-separated CORS origins (empty = localhost defaults) + secureCookies bool // set Secure flag on cookies (for TLS deployments) + version string // release version (e.g. "dev", "1.2.3") + commit string // git commit hash + buildTime string // build timestamp } func New(s *store.Store) *Server { - srv := &Server{store: s} - srv.setupRouter() - return srv + return &Server{ + store: s, + rateLimiters: NewRateLimiters(), + } } // SetVersion stores the build version info for the health endpoint. @@ -65,6 +71,16 @@ func (s *Server) SetEmailSender(e *email.Sender) { s.email = e } +// SetCORSOrigins configures allowed CORS origins (comma-separated). +func (s *Server) SetCORSOrigins(origins string) { + s.corsOrigins = origins +} + +// SetSecureCookies enables the Secure flag on all cookies. +func (s *Server) SetSecureCookies(secure bool) { + s.secureCookies = secure +} + // reconfigureEmail reads email settings from the platform_settings table // and updates (or creates) the email sender. Called after admin settings change. func (s *Server) reconfigureEmail() { @@ -95,18 +111,25 @@ func (s *Server) setupRouter() { r := chi.NewRouter() // Middleware + r.Use(chimiddleware.RealIP) r.Use(chimiddleware.Logger) r.Use(chimiddleware.Recoverer) r.Use(chimiddleware.RequestID) + r.Use(SecurityHeaders) + if s.secureCookies { + r.Use(StrictTransportSecurity) + } r.Use(cors.Handler(cors.Options{ - AllowedOrigins: []string{"http://localhost:*", "http://127.0.0.1:*"}, - AllowedMethods: []string{"GET", "POST", "PATCH", "DELETE", "OPTIONS"}, - AllowedHeaders: []string{"Accept", "Authorization", "Content-Type"}, + AllowedOrigins: parseCORSOrigins(s.corsOrigins), + AllowedMethods: []string{"GET", "POST", "PATCH", "PUT", "DELETE", "OPTIONS"}, + AllowedHeaders: []string{"Accept", "Authorization", "Content-Type", "X-CSRF-Token"}, AllowCredentials: true, MaxAge: 300, })) r.Use(s.TokenAuth) r.Use(s.SessionAuth) + r.Use(s.RateLimit) + r.Use(s.CSRFProtect) r.Use(s.RequireAuth) r.Use(jsonContentType) @@ -338,11 +361,21 @@ func (s *Server) spaHandler() http.Handler { }) } +// ensureRouter lazily initializes the router on first use, so all Set* +// configuration is applied before the middleware chain is built. +func (s *Server) ensureRouter() { + s.routerOnce.Do(func() { + s.setupRouter() + }) +} + func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { + s.ensureRouter() s.router.ServeHTTP(w, r) } func (s *Server) ListenAndServe(addr string) error { + s.ensureRouter() log.Printf("Pad server listening on %s", addr) return http.ListenAndServe(addr, s.router) } @@ -374,6 +407,14 @@ func writeError(w http.ResponseWriter, status int, code, message string) { }) } +// writeInternalError logs the real error server-side and sends a generic +// message to the client. This prevents leaking SQL errors, file paths, +// and other internal details. +func writeInternalError(w http.ResponseWriter, err error) { + log.Printf("internal error: %v", err) + writeError(w, http.StatusInternalServerError, "internal_error", "An internal error occurred") +} + func decodeJSON(r *http.Request, v interface{}) error { if err := json.NewDecoder(r.Body).Decode(v); err != nil { return fmt.Errorf("invalid JSON: %w", err) @@ -386,7 +427,7 @@ func (s *Server) getWorkspaceID(w http.ResponseWriter, r *http.Request) (string, slug := chi.URLParam(r, "slug") ws, err := s.store.GetWorkspaceBySlug(slug) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return "", false } if ws == nil { @@ -406,7 +447,7 @@ func (s *Server) getWorkspaceDocument(w http.ResponseWriter, r *http.Request) (s docID := chi.URLParam(r, "docID") doc, err := s.store.GetDocument(docID) if err != nil { - writeError(w, http.StatusInternalServerError, "internal_error", err.Error()) + writeInternalError(w, err) return "", nil, false } if doc == nil || doc.WorkspaceID != workspaceID { diff --git a/internal/store/search.go b/internal/store/search.go index fdbb8ca2..ef8bfa39 100644 --- a/internal/store/search.go +++ b/internal/store/search.go @@ -13,9 +13,19 @@ type SearchResult struct { Rank float64 `json:"rank"` } +// placeholders returns a comma-separated string of SQL placeholders: "?, ?, ?" +func placeholders(n int) string { + if n <= 0 { + return "" + } + s := strings.Repeat("?, ", n) + return s[:len(s)-2] // trim trailing ", " +} + type SearchParams struct { - Query string - Workspace string // workspace slug, optional + Query string + Workspace string // workspace slug, optional — scopes to single workspace + WorkspaceIDs []string // workspace IDs to scope results to (used when no specific workspace is given) } func (s *Store) Search(params SearchParams) ([]SearchResult, error) { @@ -43,6 +53,11 @@ func (s *Store) Search(params SearchParams) ([]SearchResult, error) { if params.Workspace != "" { refQuery += ` AND i.workspace_id = (SELECT id FROM workspaces WHERE slug = ? AND deleted_at IS NULL)` refArgs = append(refArgs, params.Workspace) + } else if len(params.WorkspaceIDs) > 0 { + refQuery += ` AND i.workspace_id IN (` + placeholders(len(params.WorkspaceIDs)) + `)` + for _, id := range params.WorkspaceIDs { + refArgs = append(refArgs, id) + } } refRows, err := s.db.Query(refQuery, refArgs...) @@ -104,6 +119,11 @@ func (s *Store) Search(params SearchParams) ([]SearchResult, error) { ) ` args = append(args, params.Workspace) + } else if len(params.WorkspaceIDs) > 0 { + query += ` AND i.workspace_id IN (` + placeholders(len(params.WorkspaceIDs)) + `)` + for _, id := range params.WorkspaceIDs { + args = append(args, id) + } } query += " ORDER BY rank LIMIT 50" diff --git a/internal/webhooks/dispatcher.go b/internal/webhooks/dispatcher.go index 938d47a0..15f0408f 100644 --- a/internal/webhooks/dispatcher.go +++ b/internal/webhooks/dispatcher.go @@ -30,8 +30,9 @@ type WebhookPayload struct { // Dispatcher sends webhook HTTP POST notifications for workspace events. type Dispatcher struct { - store WebhookStore - client *http.Client + store WebhookStore + client *http.Client + SkipSSRF bool // Skip SSRF validation (for tests only) } // NewDispatcher creates a Dispatcher with the given store. @@ -79,6 +80,15 @@ func (d *Dispatcher) Dispatch(workspaceID, event string, data interface{}) { // deliver sends a single HTTP POST to the webhook URL. func (d *Dispatcher) deliver(hook models.Webhook, body []byte) { + // Defense in depth: re-validate URL before making the request + if !d.SkipSSRF { + if err := ValidateWebhookURL(hook.URL); err != nil { + log.Printf("webhooks: blocked delivery to %s: %v", hook.URL, err) + d.store.UpdateWebhookFailure(hook.ID, true) + return + } + } + req, err := http.NewRequest(http.MethodPost, hook.URL, bytes.NewReader(body)) if err != nil { log.Printf("webhooks: failed to create request for %s: %v", hook.URL, err) diff --git a/internal/webhooks/dispatcher_test.go b/internal/webhooks/dispatcher_test.go index 6abf5c05..c5d781db 100644 --- a/internal/webhooks/dispatcher_test.go +++ b/internal/webhooks/dispatcher_test.go @@ -80,6 +80,7 @@ func TestDispatcher_Dispatch(t *testing.T) { }) d := NewDispatcher(store) + d.SkipSSRF = true d.Dispatch("ws-1", "item.created", map[string]string{"title": "Test Item"}) store.waitForUpdate() @@ -124,6 +125,7 @@ func TestDispatcher_EventFiltering(t *testing.T) { }) d := NewDispatcher(store) + d.SkipSSRF = true // This event should NOT match — no goroutine launched, no store update d.Dispatch("ws-1", "item.deleted", map[string]string{"title": "Test"}) @@ -156,6 +158,7 @@ func TestDispatcher_WildcardEvent(t *testing.T) { }) d := NewDispatcher(store) + d.SkipSSRF = true d.Dispatch("ws-1", "item.deleted", map[string]string{"title": "Test"}) store.waitForUpdate() @@ -189,6 +192,7 @@ func TestDispatcher_InactiveWebhookSkipped(t *testing.T) { }) d := NewDispatcher(store) + d.SkipSSRF = true d.Dispatch("ws-1", "item.created", map[string]string{"title": "Test"}) // Since the hook is inactive, no goroutine is launched @@ -214,6 +218,7 @@ func TestDispatcher_FailureOnNon2xx(t *testing.T) { }) d := NewDispatcher(store) + d.SkipSSRF = true d.Dispatch("ws-1", "item.created", map[string]string{"title": "Test"}) store.waitForUpdate() diff --git a/internal/webhooks/validate.go b/internal/webhooks/validate.go new file mode 100644 index 00000000..78b3c58e --- /dev/null +++ b/internal/webhooks/validate.go @@ -0,0 +1,107 @@ +package webhooks + +import ( + "fmt" + "net" + "net/url" + "strings" +) + +// ValidateWebhookURL checks that a webhook URL is safe to call. +// It rejects non-HTTP(S) schemes, URLs with credentials, private/reserved +// IPs (loopback, link-local, RFC1918, cloud metadata), and hostnames that +// resolve to private IPs. +func ValidateWebhookURL(rawURL string) error { + u, err := url.Parse(rawURL) + if err != nil { + return fmt.Errorf("invalid URL: %w", err) + } + + // Scheme must be http or https + switch u.Scheme { + case "http", "https": + // ok + default: + return fmt.Errorf("unsupported scheme %q: only http and https are allowed", u.Scheme) + } + + // Reject URLs with embedded credentials + if u.User != nil { + return fmt.Errorf("URLs with embedded credentials are not allowed") + } + + host := u.Hostname() + if host == "" { + return fmt.Errorf("URL must have a hostname") + } + + // Check if host is a literal IP + if ip := net.ParseIP(host); ip != nil { + if isPrivateIP(ip) { + return fmt.Errorf("webhook URLs must not target private or reserved IP addresses") + } + return nil + } + + // Host is a name — resolve it and check all resulting IPs + ips, err := net.LookupIP(host) + if err != nil { + return fmt.Errorf("failed to resolve hostname %q: %w", host, err) + } + for _, ip := range ips { + if isPrivateIP(ip) { + return fmt.Errorf("hostname %q resolves to private/reserved IP %s", host, ip) + } + } + + return nil +} + +// isPrivateIP returns true if the IP is in a private, reserved, or +// otherwise non-routable range. +func isPrivateIP(ip net.IP) bool { + // Loopback (127.0.0.0/8, ::1) + if ip.IsLoopback() { + return true + } + + // Link-local (169.254.0.0/16, fe80::/10) + if ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() { + return true + } + + // Unspecified (0.0.0.0, ::) + if ip.IsUnspecified() { + return true + } + + // RFC1918 private ranges + privateRanges := []struct { + network string + }{ + {"10.0.0.0/8"}, + {"172.16.0.0/12"}, + {"192.168.0.0/16"}, + // IPv6 unique local (fc00::/7) + {"fc00::/7"}, + // Cloud metadata (AWS, GCP, Azure) + {"169.254.169.254/32"}, + } + + for _, r := range privateRanges { + _, cidr, err := net.ParseCIDR(r.network) + if err != nil { + continue + } + if cidr.Contains(ip) { + return true + } + } + + // Also catch common cloud metadata IPv6 variants + if strings.EqualFold(ip.String(), "fd00::") { + return true + } + + return false +} diff --git a/internal/webhooks/validate_test.go b/internal/webhooks/validate_test.go new file mode 100644 index 00000000..c5d1f980 --- /dev/null +++ b/internal/webhooks/validate_test.go @@ -0,0 +1,86 @@ +package webhooks + +import ( + "net" + "testing" +) + +func TestValidateWebhookURL(t *testing.T) { + tests := []struct { + name string + url string + wantErr bool + }{ + // Valid URLs + {"valid https", "https://example.com/webhook", false}, + {"valid http", "http://example.com/callback", false}, + {"valid with port", "https://example.com:8080/hook", false}, + {"valid with path", "https://example.com/api/v1/webhook", false}, + + // Invalid schemes + {"ftp scheme", "ftp://example.com/hook", true}, + {"javascript scheme", "javascript:alert(1)", true}, + {"file scheme", "file:///etc/passwd", true}, + {"no scheme", "example.com/hook", true}, + + // Embedded credentials + {"with credentials", "https://user:pass@example.com/hook", true}, + + // Private IPs + {"loopback IPv4", "http://127.0.0.1/hook", true}, + {"loopback IPv6", "http://[::1]/hook", true}, + {"private 10.x", "http://10.0.0.1/hook", true}, + {"private 172.16.x", "http://172.16.0.1/hook", true}, + {"private 192.168.x", "http://192.168.1.1/hook", true}, + {"cloud metadata", "http://169.254.169.254/latest/meta-data/", true}, + {"link-local", "http://169.254.1.1/hook", true}, + {"unspecified", "http://0.0.0.0/hook", true}, + + // Hostnames resolving to private IPs + {"localhost", "http://localhost/hook", true}, + + // Empty/invalid + {"empty url", "", true}, + {"no host", "http:///path", true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateWebhookURL(tt.url) + if (err != nil) != tt.wantErr { + t.Errorf("ValidateWebhookURL(%q) error = %v, wantErr = %v", tt.url, err, tt.wantErr) + } + }) + } +} + +func TestIsPrivateIP(t *testing.T) { + tests := []struct { + ip string + private bool + }{ + {"127.0.0.1", true}, + {"10.0.0.1", true}, + {"172.16.0.1", true}, + {"192.168.0.1", true}, + {"169.254.169.254", true}, + {"0.0.0.0", true}, + {"::1", true}, + {"8.8.8.8", false}, + {"1.1.1.1", false}, + {"93.184.216.34", false}, + } + + for _, tt := range tests { + t.Run(tt.ip, func(t *testing.T) { + ip := net.ParseIP(tt.ip) + if ip == nil { + t.Fatalf("failed to parse IP: %s", tt.ip) + } + got := isPrivateIP(ip) + if got != tt.private { + t.Errorf("isPrivateIP(%s) = %v, want %v", tt.ip, got, tt.private) + } + }) + } +} diff --git a/web/src/lib/api/client.ts b/web/src/lib/api/client.ts index c1b6867f..12d736f4 100644 --- a/web/src/lib/api/client.ts +++ b/web/src/lib/api/client.ts @@ -45,9 +45,24 @@ class PadApiError extends Error { } } +function getCSRFToken(): string | null { + if (typeof document === 'undefined') return null; + const match = document.cookie.match(/(?:^|;\s*)pad_csrf=([^;]+)/); + return match ? match[1] : null; +} + async function request(path: string, options?: RequestInit): Promise { + const headers: Record = { 'Content-Type': 'application/json' }; + + // Attach CSRF token for state-changing requests + const method = options?.method?.toUpperCase(); + if (method && method !== 'GET' && method !== 'HEAD') { + const csrf = getCSRFToken(); + if (csrf) headers['X-CSRF-Token'] = csrf; + } + const resp = await fetch(BASE + path, { - headers: { 'Content-Type': 'application/json' }, + headers, credentials: 'same-origin', ...options });