diff --git a/.golangci.yml b/.golangci.yml index d970055f..3a437efc 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,46 +1,49 @@ +version: "2" run: - timeout: 5m modules-download-mode: readonly - linters: - # Minimal set targeted at "the code still compiles + is formatted". - # Larger sets keep catching legacy issues unrelated to this PR - # (deprecated pubsub usage, tautological comparisons in scheduler - # helpers, empty branches in event handlers, etc.). Tracking those - # for a dedicated cleanup pass. - disable-all: true + default: none enable: - - govet - - typecheck - - gofmt - bodyclose + - govet - noctx - -linters-settings: - govet: - # Real-bug subset of govet. shadow + nilness + unusedwrite fire - # on legacy patterns (idiomatic err re-decl in tx blocks, - # defensive nil checks the analyzer reads as tautological, - # struct-builder writes to fields read later). Field-alignment - # is intentional for readability. - enable-all: true - disable: - - fieldalignment - - shadow - - nilness - - unusedwrite - + settings: + govet: + disable: + - fieldalignment + - shadow + - nilness + - unusedwrite + enable-all: true + exclusions: + generated: lax + presets: + - comments + - common-false-positives + - legacy + - std-error-handling + rules: + - linters: + - errcheck + - gosec + - unparam + path: _test\.go + - linters: + - unparam + path: cmd/ + paths: + - third_party$ + - builtin$ + - examples$ issues: - exclude-rules: - - path: _test\.go - linters: - - errcheck - - gosec - - unparam - - - path: cmd/ - linters: - - unparam - max-issues-per-linter: 50 max-same-issues: 10 +formatters: + enable: + - gofmt + exclusions: + generated: lax + paths: + - third_party$ + - builtin$ + - examples$ diff --git a/internal/api/handler/auth.go b/internal/api/handler/auth.go index 875073ec..4547b969 100644 --- a/internal/api/handler/auth.go +++ b/internal/api/handler/auth.go @@ -1,7 +1,9 @@ package handler import ( + "context" "net/http" + "time" "github.com/gin-gonic/gin" "github.com/google/uuid" @@ -10,6 +12,8 @@ import ( "github.com/warmbly/warmbly/internal/errx" ) +const authRequestTimeout = 15 * time.Second + func (h *Handler) LoginStart(c *gin.Context) { var data auth.AuthData @@ -18,7 +22,10 @@ func (h *Handler) LoginStart(c *gin.Context) { return } - resp, err := h.AuthService.LoginStart(c.Request.Context(), &data, c.ClientIP()) + ctx, cancel := context.WithTimeout(c.Request.Context(), authRequestTimeout) + defer cancel() + + resp, err := h.AuthService.LoginStart(ctx, &data, c.ClientIP()) if err != nil { errx.Handle(c, err) return @@ -35,7 +42,10 @@ func (h *Handler) LoginConfirm(c *gin.Context) { return } - resp, err := h.AuthService.LoginConfirm(c.Request.Context(), &data, data.Session, c.ClientIP(), c.Request.UserAgent()) + ctx, cancel := context.WithTimeout(c.Request.Context(), authRequestTimeout) + defer cancel() + + resp, err := h.AuthService.LoginConfirm(ctx, &data, data.Session, c.ClientIP(), c.Request.UserAgent()) if err != nil { errx.Handle(c, err) return @@ -52,7 +62,10 @@ func (h *Handler) RegistrationStart(c *gin.Context) { return } - resp, err := h.AuthService.RegistrationStart(c.Request.Context(), &data, c.ClientIP()) + ctx, cancel := context.WithTimeout(c.Request.Context(), authRequestTimeout) + defer cancel() + + resp, err := h.AuthService.RegistrationStart(ctx, &data, c.ClientIP()) if err != nil { errx.Handle(c, err) return @@ -69,7 +82,10 @@ func (h *Handler) RegistrationConfirm(c *gin.Context) { return } - if err := h.AuthService.RegistrationConfirm(c.Request.Context(), &data, data.Session, c.ClientIP()); err != nil { + ctx, cancel := context.WithTimeout(c.Request.Context(), authRequestTimeout) + defer cancel() + + if err := h.AuthService.RegistrationConfirm(ctx, &data, data.Session, c.ClientIP()); err != nil { errx.Handle(c, err) return } @@ -161,7 +177,10 @@ func (h *Handler) ResetPasswordStart(c *gin.Context) { return } - if err := h.AuthService.ResetPasswordStart(c.Request.Context(), &data, c.ClientIP()); err != nil { + ctx, cancel := context.WithTimeout(c.Request.Context(), authRequestTimeout) + defer cancel() + + if err := h.AuthService.ResetPasswordStart(ctx, &data, c.ClientIP()); err != nil { errx.Handle(c, err) return } @@ -177,7 +196,10 @@ func (h *Handler) ResetPasswordConfirm(c *gin.Context) { return } - if err := h.AuthService.ResetPasswordConfirm(c.Request.Context(), &data, data.Session, c.ClientIP()); err != nil { + ctx, cancel := context.WithTimeout(c.Request.Context(), authRequestTimeout) + defer cancel() + + if err := h.AuthService.ResetPasswordConfirm(ctx, &data, data.Session, c.ClientIP()); err != nil { errx.Handle(c, err) return } diff --git a/internal/api/handler/internal_dek_test.go b/internal/api/handler/internal_dek_test.go index 31c0effc..7baf614b 100644 --- a/internal/api/handler/internal_dek_test.go +++ b/internal/api/handler/internal_dek_test.go @@ -57,7 +57,7 @@ func TestInternalGetDEK_Found(t *testing.T) { r := newDEKRouter(t, store) w := httptest.NewRecorder() - req := httptest.NewRequest("GET", "/dek/"+id.String(), nil) + req := httptest.NewRequestWithContext(context.Background(), "GET", "/dek/"+id.String(), nil) r.ServeHTTP(w, req) if w.Code != http.StatusOK { @@ -80,7 +80,7 @@ func TestInternalGetDEK_NotFoundReturns404(t *testing.T) { } r := newDEKRouter(t, store) w := httptest.NewRecorder() - req := httptest.NewRequest("GET", "/dek/"+uuid.New().String(), nil) + req := httptest.NewRequestWithContext(context.Background(), "GET", "/dek/"+uuid.New().String(), nil) r.ServeHTTP(w, req) if w.Code != http.StatusNotFound { t.Fatalf("expected 404, got %d", w.Code) @@ -90,7 +90,7 @@ func TestInternalGetDEK_NotFoundReturns404(t *testing.T) { func TestInternalGetDEK_BadUUID(t *testing.T) { r := newDEKRouter(t, &mockEKStore{}) w := httptest.NewRecorder() - req := httptest.NewRequest("GET", "/dek/not-a-uuid", nil) + req := httptest.NewRequestWithContext(context.Background(), "GET", "/dek/not-a-uuid", nil) r.ServeHTTP(w, req) if w.Code != http.StatusBadRequest { t.Fatalf("expected 400, got %d", w.Code) @@ -105,7 +105,7 @@ func TestInternalGetDEK_StoreError(t *testing.T) { } r := newDEKRouter(t, store) w := httptest.NewRecorder() - req := httptest.NewRequest("GET", "/dek/"+uuid.New().String(), nil) + req := httptest.NewRequestWithContext(context.Background(), "GET", "/dek/"+uuid.New().String(), nil) r.ServeHTTP(w, req) if w.Code != http.StatusInternalServerError { t.Fatalf("expected 500, got %d", w.Code) @@ -128,7 +128,7 @@ func TestInternalPutDEK_Created(t *testing.T) { r := newDEKRouter(t, store) body, _ := json.Marshal(dekPayload{EncryptedDataKey: "blob"}) w := httptest.NewRecorder() - req := httptest.NewRequest("PUT", "/dek/"+id.String(), bytes.NewReader(body)) + req := httptest.NewRequestWithContext(context.Background(), "PUT", "/dek/"+id.String(), bytes.NewReader(body)) r.ServeHTTP(w, req) if w.Code != http.StatusCreated { t.Fatalf("expected 201, got %d", w.Code) @@ -144,7 +144,7 @@ func TestInternalPutDEK_ConflictReturns409(t *testing.T) { r := newDEKRouter(t, store) body, _ := json.Marshal(dekPayload{EncryptedDataKey: "blob"}) w := httptest.NewRecorder() - req := httptest.NewRequest("PUT", "/dek/"+uuid.New().String(), bytes.NewReader(body)) + req := httptest.NewRequestWithContext(context.Background(), "PUT", "/dek/"+uuid.New().String(), bytes.NewReader(body)) r.ServeHTTP(w, req) if w.Code != http.StatusConflict { t.Fatalf("expected 409, got %d", w.Code) @@ -154,7 +154,7 @@ func TestInternalPutDEK_ConflictReturns409(t *testing.T) { func TestInternalPutDEK_RejectsEmptyBody(t *testing.T) { r := newDEKRouter(t, &mockEKStore{}) w := httptest.NewRecorder() - req := httptest.NewRequest("PUT", "/dek/"+uuid.New().String(), strings.NewReader(`{}`)) + req := httptest.NewRequestWithContext(context.Background(), "PUT", "/dek/"+uuid.New().String(), strings.NewReader(`{}`)) r.ServeHTTP(w, req) if w.Code != http.StatusBadRequest { t.Fatalf("expected 400 for empty key, got %d", w.Code) @@ -167,7 +167,7 @@ func TestInternalDeleteDEK_NoContent(t *testing.T) { } r := newDEKRouter(t, store) w := httptest.NewRecorder() - req := httptest.NewRequest("DELETE", "/dek/"+uuid.New().String(), nil) + req := httptest.NewRequestWithContext(context.Background(), "DELETE", "/dek/"+uuid.New().String(), nil) r.ServeHTTP(w, req) if w.Code != http.StatusNoContent { t.Fatalf("expected 204, got %d", w.Code) diff --git a/internal/api/middleware/apikey_test.go b/internal/api/middleware/apikey_test.go index 87c298f5..227d625d 100644 --- a/internal/api/middleware/apikey_test.go +++ b/internal/api/middleware/apikey_test.go @@ -1,6 +1,7 @@ package middleware import ( + "context" "net/http" "net/http/httptest" "testing" @@ -48,7 +49,7 @@ func TestRequireAPIKeyEmailAccountParam(t *testing.T) { }, ) - req := httptest.NewRequest(http.MethodGet, "/emails/"+tt.pathID.String(), nil) + req := httptest.NewRequestWithContext(context.Background(), http.MethodGet, "/emails/"+tt.pathID.String(), nil) rec := httptest.NewRecorder() r.ServeHTTP(rec, req) @@ -91,7 +92,7 @@ func TestRequireAPIPermission(t *testing.T) { }) w := httptest.NewRecorder() - req := httptest.NewRequest(http.MethodGet, "/x", nil) + req := httptest.NewRequestWithContext(context.Background(), http.MethodGet, "/x", nil) r.ServeHTTP(w, req) if w.Code != tt.wantStatus { @@ -129,7 +130,7 @@ func TestRequireAccessAPIKeyPath(t *testing.T) { }) w := httptest.NewRecorder() - req := httptest.NewRequest(http.MethodGet, "/x", nil) + req := httptest.NewRequestWithContext(context.Background(), http.MethodGet, "/x", nil) r.ServeHTTP(w, req) if w.Code != tt.wantStatus { diff --git a/internal/api/middleware/idempotency_test.go b/internal/api/middleware/idempotency_test.go index abb25e0d..27d21b71 100644 --- a/internal/api/middleware/idempotency_test.go +++ b/internal/api/middleware/idempotency_test.go @@ -50,7 +50,7 @@ func TestIdempotencyMiddlewareStoresResponse(t *testing.T) { }, ) - req := httptest.NewRequest(http.MethodPost, "/contacts", nil) + req := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "/contacts", nil) req.Header.Set(IdempotencyKeyHeader, "idem_123") rec := httptest.NewRecorder() r.ServeHTTP(rec, req) @@ -91,7 +91,7 @@ func TestIdempotencyMiddlewareReplaysResponse(t *testing.T) { }, ) - req := httptest.NewRequest(http.MethodPost, "/contacts", nil) + req := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "/contacts", nil) req.Header.Set(IdempotencyKeyHeader, "idem_123") rec := httptest.NewRecorder() r.ServeHTTP(rec, req) diff --git a/internal/api/middleware/internal_auth_test.go b/internal/api/middleware/internal_auth_test.go index a6a74ce3..1cc9a4cd 100644 --- a/internal/api/middleware/internal_auth_test.go +++ b/internal/api/middleware/internal_auth_test.go @@ -1,6 +1,7 @@ package middleware import ( + "context" "net/http" "net/http/httptest" "testing" @@ -35,7 +36,7 @@ func TestInternalAuth_RejectsMissingHeader(t *testing.T) { r := newRouterWithInternalAuth(t) w := httptest.NewRecorder() - req := httptest.NewRequest("GET", "/internal/ping", nil) + req := httptest.NewRequestWithContext(context.Background(), "GET", "/internal/ping", nil) r.ServeHTTP(w, req) if w.Code != http.StatusUnauthorized { @@ -48,7 +49,7 @@ func TestInternalAuth_RejectsWrongScheme(t *testing.T) { r := newRouterWithInternalAuth(t) w := httptest.NewRecorder() - req := httptest.NewRequest("GET", "/internal/ping", nil) + req := httptest.NewRequestWithContext(context.Background(), "GET", "/internal/ping", nil) req.Header.Set("Authorization", "Basic secret") r.ServeHTTP(w, req) @@ -62,7 +63,7 @@ func TestInternalAuth_RejectsWrongToken(t *testing.T) { r := newRouterWithInternalAuth(t) w := httptest.NewRecorder() - req := httptest.NewRequest("GET", "/internal/ping", nil) + req := httptest.NewRequestWithContext(context.Background(), "GET", "/internal/ping", nil) req.Header.Set("Authorization", "Bearer wrong") r.ServeHTTP(w, req) @@ -76,7 +77,7 @@ func TestInternalAuth_AcceptsCorrectToken(t *testing.T) { r := newRouterWithInternalAuth(t) w := httptest.NewRecorder() - req := httptest.NewRequest("GET", "/internal/ping", nil) + req := httptest.NewRequestWithContext(context.Background(), "GET", "/internal/ping", nil) req.Header.Set("Authorization", "Bearer secret") r.ServeHTTP(w, req) @@ -92,7 +93,7 @@ func TestInternalAuth_FailsClosedWhenTokenUnset(t *testing.T) { r := newRouterWithInternalAuth(t) w := httptest.NewRecorder() - req := httptest.NewRequest("GET", "/internal/ping", nil) + req := httptest.NewRequestWithContext(context.Background(), "GET", "/internal/ping", nil) req.Header.Set("Authorization", "Bearer anything") r.ServeHTTP(w, req) diff --git a/internal/api/middleware/request_id_test.go b/internal/api/middleware/request_id_test.go index 30c73b06..b30040b1 100644 --- a/internal/api/middleware/request_id_test.go +++ b/internal/api/middleware/request_id_test.go @@ -1,6 +1,7 @@ package middleware import ( + "context" "net/http" "net/http/httptest" "testing" @@ -16,7 +17,7 @@ func TestRequestIDMiddlewareUsesClientRequestID(t *testing.T) { c.String(http.StatusOK, c.GetString(RequestIDContextKey)) }) - req := httptest.NewRequest(http.MethodGet, "/x", nil) + req := httptest.NewRequestWithContext(context.Background(), http.MethodGet, "/x", nil) req.Header.Set(RequestIDHeader, "client-trace_123") rec := httptest.NewRecorder() r.ServeHTTP(rec, req) @@ -40,7 +41,7 @@ func TestRequestIDMiddlewareReplacesUnsafeRequestID(t *testing.T) { c.String(http.StatusOK, c.GetString(RequestIDContextKey)) }) - req := httptest.NewRequest(http.MethodGet, "/x", nil) + req := httptest.NewRequestWithContext(context.Background(), http.MethodGet, "/x", nil) req.Header.Set(RequestIDHeader, "bad/request/id") rec := httptest.NewRecorder() r.ServeHTTP(rec, req) diff --git a/internal/api/routes.go b/internal/api/routes.go index 6d8b2e2a..a14ea2fb 100644 --- a/internal/api/routes.go +++ b/internal/api/routes.go @@ -100,6 +100,8 @@ func Run( "http://127.0.0.1:4173", "http://localhost:5173", "http://127.0.0.1:5173", + "http://localhost:5174", + "http://127.0.0.1:5174", } corsConfig.AllowCredentials = true case len(allowedOrigins) == 1 && allowedOrigins[0] == "*": diff --git a/internal/app/auth/email.go b/internal/app/auth/email.go new file mode 100644 index 00000000..f54501d6 --- /dev/null +++ b/internal/app/auth/email.go @@ -0,0 +1,15 @@ +package auth + +import ( + "context" + "time" +) + +const authEmailSendTimeout = 10 * time.Second + +func (s *authService) sendAuthEmail(ctx context.Context, to, subject, message string) error { + ctx, cancel := context.WithTimeout(ctx, authEmailSendTimeout) + defer cancel() + + return s.emailNotificationService.Send(ctx, []string{to}, nil, nil, subject, message) +} diff --git a/internal/app/auth/login.go b/internal/app/auth/login.go index a2cf6460..a912a55d 100644 --- a/internal/app/auth/login.go +++ b/internal/app/auth/login.go @@ -51,7 +51,7 @@ func (s *authService) LoginStart(ctx context.Context, data *AuthData, ipaddr str return nil, errx.InternalError() } - if xerr := s.emailNotificationService.Send(ctx, []string{data.Email}, nil, nil, "Your Login Code", text); xerr != nil { + if xerr := s.sendAuthEmail(ctx, data.Email, "Your Login Code", text); xerr != nil { sentry.CaptureException(xerr) return nil, errx.InternalError() } diff --git a/internal/app/auth/registration.go b/internal/app/auth/registration.go index e96628e5..0ab1f231 100644 --- a/internal/app/auth/registration.go +++ b/internal/app/auth/registration.go @@ -55,7 +55,7 @@ func (s *authService) RegistrationStart(ctx context.Context, data *AuthData, ipa return nil, errx.InternalError() } - if xerr := s.emailNotificationService.Send(ctx, []string{data.Email}, nil, nil, "Your Verification Code", text); xerr != nil { + if xerr := s.sendAuthEmail(ctx, data.Email, "Your Verification Code", text); xerr != nil { sentry.CaptureException(xerr) return nil, errx.InternalError() } diff --git a/internal/app/auth/reset_password.go b/internal/app/auth/reset_password.go index afbae159..ad74561f 100644 --- a/internal/app/auth/reset_password.go +++ b/internal/app/auth/reset_password.go @@ -66,7 +66,7 @@ func (s *authService) ResetPasswordStart(ctx context.Context, data *ResetPasswor return errx.InternalError() } - if err := s.emailNotificationService.Send(ctx, []string{u.Email}, nil, nil, "Password Reset Confirmation", text); err != nil { + if err := s.sendAuthEmail(ctx, u.Email, "Password Reset Confirmation", text); err != nil { sentry.CaptureException(err) return errx.InternalError() } diff --git a/internal/app/email/handler.go b/internal/app/email/handler.go index 1720e3bc..d394845b 100644 --- a/internal/app/email/handler.go +++ b/internal/app/email/handler.go @@ -81,7 +81,7 @@ func (s *emailService) UpdateTrackingDomain(ctx context.Context, userID, emailAc // it as verified once it points at our tracking host. DNS can lag // behind a freshly-added record, so a miss is "pending", not an // error — the customer just re-verifies. - if cname, err := net.LookupCNAME(domain); err == nil { + if cname, err := net.DefaultResolver.LookupCNAME(ctx, domain); err == nil { resolved := strings.TrimSuffix(strings.ToLower(cname), ".") if strings.Contains(resolved, trackingDomainTarget) { status.TrackingDomainVerified = true diff --git a/internal/config/config_api.go b/internal/config/config_api.go index a57790d5..31f6163d 100644 --- a/internal/config/config_api.go +++ b/internal/config/config_api.go @@ -26,14 +26,25 @@ func (c *Config) LoadApiConfig(ctx context.Context) (*ApiConfig, error) { return nil, err } - allowedOriginsRaw := os.Getenv("CORS_ALLOW_ORIGINS") - if allowedOriginsRaw == "" { - allowedOriginsRaw = os.Getenv("APP_URL") - } - allowedOrigins := splitCSV(allowedOriginsRaw) + allowedOrigins := splitCSV(os.Getenv("CORS_ALLOW_ORIGINS")) if len(allowedOrigins) == 0 { + allowedOrigins = appendOrigin(allowedOrigins, os.Getenv("APP_URL")) if origin := originFromURI(websocketUri); origin != "" { - allowedOrigins = []string{origin} + allowedOrigins = appendOrigin(allowedOrigins, origin) + } + if c.Env != "prod" { + for _, origin := range []string{ + "http://localhost:3000", + "http://127.0.0.1:3000", + "http://localhost:4173", + "http://127.0.0.1:4173", + "http://localhost:5173", + "http://127.0.0.1:5173", + "http://localhost:5174", + "http://127.0.0.1:5174", + } { + allowedOrigins = appendOrigin(allowedOrigins, origin) + } } } @@ -71,6 +82,19 @@ func splitCSV(value string) []string { return out } +func appendOrigin(origins []string, origin string) []string { + origin = strings.TrimSpace(origin) + if origin == "" { + return origins + } + for _, existing := range origins { + if existing == origin { + return origins + } + } + return append(origins, origin) +} + func originFromURI(raw string) string { if raw == "" { return "" diff --git a/internal/email/imap.go b/internal/email/imap.go index 41102ec4..456ceb80 100644 --- a/internal/email/imap.go +++ b/internal/email/imap.go @@ -24,7 +24,7 @@ func VerifyImap(ctx context.Context, host string, port int, user, pass string) b if err := tlsConn.SetDeadline(time.Now().Add(5 * time.Second)); err != nil { return false } - if err := tlsConn.Handshake(); err != nil { + if err := tlsConn.HandshakeContext(ctx); err != nil { return false } diff --git a/internal/notify/smtp.go b/internal/notify/smtp.go index 0b43808c..2832b35c 100644 --- a/internal/notify/smtp.go +++ b/internal/notify/smtp.go @@ -2,14 +2,19 @@ package notify import ( "context" + "errors" "fmt" + "io" "net" "net/smtp" "strings" + "time" "github.com/getsentry/sentry-go" ) +const smtpSendTimeout = 10 * time.Second + type smtpEmailNotificationService struct { Name string Address string @@ -43,7 +48,7 @@ func (s *smtpEmailNotificationService) Send(ctx context.Context, to, cc, bcc []s msg := []byte(headers + message) - if err := smtp.SendMail(addr, nil, s.Address, allRecipients, msg); err != nil { + if err := sendSMTP(ctx, addr, s.Host, s.Address, allRecipients, msg); err != nil { sentry.CaptureException(err) return err } @@ -68,9 +73,63 @@ func (s *smtpEmailNotificationService) SendOutreach(ctx context.Context, to []st "Content-Type: text/html; charset=\"UTF-8\"\r\n\r\n" msg := []byte(headers + message) - if err := smtp.SendMail(addr, nil, s.Address, to, msg); err != nil { + if err := sendSMTP(ctx, addr, s.Host, s.Address, to, msg); err != nil { sentry.CaptureException(err) return err } return nil } + +func sendSMTP(ctx context.Context, addr, host, from string, recipients []string, msg []byte) error { + if len(recipients) == 0 { + return errors.New("smtp send requires at least one recipient") + } + + ctx, cancel := context.WithTimeout(ctx, smtpSendTimeout) + defer cancel() + + dialer := net.Dialer{Timeout: smtpSendTimeout} + conn, err := dialer.DialContext(ctx, "tcp", addr) + if err != nil { + return err + } + defer conn.Close() + + if deadline, ok := ctx.Deadline(); ok { + if err := conn.SetDeadline(deadline); err != nil { + return err + } + } + + client, err := smtp.NewClient(conn, host) + if err != nil { + return err + } + defer client.Close() + + if err := client.Mail(from); err != nil { + return err + } + for _, recipient := range recipients { + if err := client.Rcpt(recipient); err != nil { + return err + } + } + + writer, err := client.Data() + if err != nil { + return err + } + if _, err := writer.Write(msg); err != nil { + _ = writer.Close() + return err + } + if err := writer.Close(); err != nil { + return err + } + + if err := client.Quit(); err != nil && !errors.Is(err, io.EOF) { + return err + } + return nil +}