fix: address Turnstile review feedback

GeorgeXie2333 · GeorgeXie2333 · commit 884a9aafcc10 · 2026-05-25T17:43:58.000+08:00
diff --git a/backend/README.md b/backend/README.md
@@ -109,6 +109,7 @@ observability:
 - `auth:turnstile_registration_enabled`：是否在邮箱注册时启用 Turnstile。
 - `auth:turnstile_site_key`：前端渲染 Turnstile 组件使用的 Site Key，会通过 `/api/v1/auth/login-options` 返回。
 - `auth:turnstile_secret_key`：后端调用 Cloudflare siteverify 使用的 Secret Key，属于敏感设置。
+- `TURNSTILE_SITEVERIFY_URL` / `security.turnstile_siteverify_url`：可选覆盖 siteverify 端点，默认使用 Cloudflare 官方地址。
 
 启用 Turnstile 需要同时启用 `auth:email_registration_enabled`，并配置 Site Key 与 Secret Key。开启邮箱验证码注册时，前端在 `/api/v1/auth/register/email/start` 提交 `turnstileToken`；关闭邮箱验证码但允许邮箱注册时，前端在 `/api/v1/auth/register/email/complete` 提交 `turnstileToken`。
 
diff --git a/backend/internal/application/auth/registration.go b/backend/internal/application/auth/registration.go
@@ -161,6 +161,8 @@ func (s *Service) RegisterWithEmail(ctx context.Context, email string, password
 	if err != nil {
 		return nil, err
 	}
+	// With email verification enabled, Turnstile is checked before issuing the registration code.
+	// Without that step, completion is the first registration write path and must verify it here.
 	if !cfg.EmailVerificationEnabled {
 		if err = s.verifyRegistrationTurnstile(ctx, cfg, turnstileToken, remoteIP); err != nil {
 			return nil, err
diff --git a/backend/internal/application/auth/registration_test.go b/backend/internal/application/auth/registration_test.go
@@ -2,6 +2,8 @@ package auth
 
 import (
 	"context"
+	"net/http"
+	"net/http/httptest"
 	"strings"
 	"testing"
 	"time"
@@ -196,6 +198,63 @@ func TestRequestEmailRegistrationRequiresTurnstileWhenEnabled(t *testing.T) {
 	}
 }
 
+func TestVerifyRegistrationTurnstileSkipsWhenSiteKeyEmpty(t *testing.T) {
+	service := NewService(config.Config{}, nil, nil)
+
+	err := service.verifyRegistrationTurnstile(context.Background(), config.Config{
+		TurnstileRegistrationEnabled: true,
+		TurnstileSecretKey:           "secret-key",
+	}, "", "")
+	if err != nil {
+		t.Fatalf("expected empty site key to skip turnstile, got %v", err)
+	}
+}
+
+func TestVerifyRegistrationTurnstileRequiresSecretWhenSiteKeyPresent(t *testing.T) {
+	service := NewService(config.Config{}, nil, nil)
+
+	err := service.verifyRegistrationTurnstile(context.Background(), config.Config{
+		TurnstileRegistrationEnabled: true,
+		TurnstileSiteKey:             "site-key",
+	}, "token", "")
+	if err == nil || err.Error() != "turnstile is not configured" {
+		t.Fatalf("expected missing secret error, got %v", err)
+	}
+}
+
+func TestVerifyRegistrationTurnstileUsesConfiguredEndpoint(t *testing.T) {
+	var gotRemoteIP string
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.Method != http.MethodPost {
+			t.Fatalf("expected POST, got %s", r.Method)
+		}
+		if err := r.ParseForm(); err != nil {
+			t.Fatalf("parse turnstile form: %v", err)
+		}
+		if r.Form.Get("secret") != "secret-key" || r.Form.Get("response") != "token" {
+			t.Fatalf("unexpected form: %#v", r.Form)
+		}
+		gotRemoteIP = r.Form.Get("remoteip")
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = w.Write([]byte(`{"success":true}`))
+	}))
+	defer server.Close()
+
+	service := NewService(config.Config{}, nil, nil)
+	err := service.verifyRegistrationTurnstile(context.Background(), config.Config{
+		TurnstileRegistrationEnabled: true,
+		TurnstileSiteKey:             "site-key",
+		TurnstileSecretKey:           "secret-key",
+		TurnstileSiteverifyURL:       server.URL,
+	}, "token", "203.0.113.7")
+	if err != nil {
+		t.Fatalf("expected configured endpoint verification to pass, got %v", err)
+	}
+	if gotRemoteIP != "203.0.113.7" {
+		t.Fatalf("expected remote ip to be forwarded, got %q", gotRemoteIP)
+	}
+}
+
 func TestRegisterWithEmailRequiresTurnstileWhenEmailVerificationDisabled(t *testing.T) {
 	service := NewService(config.Config{
 		EmailLoginEnabled:            true,
@@ -212,6 +271,22 @@ func TestRegisterWithEmailRequiresTurnstileWhenEmailVerificationDisabled(t *test
 	}
 }
 
+func TestRegisterWithEmailDoesNotRequireTurnstileWhenEmailVerificationEnabled(t *testing.T) {
+	service := NewService(config.Config{
+		EmailLoginEnabled:            true,
+		EmailRegistrationEnabled:     true,
+		EmailVerificationEnabled:     true,
+		TurnstileRegistrationEnabled: true,
+		TurnstileSiteKey:             "site-key",
+		TurnstileSecretKey:           "secret-key",
+	}, &emailRegistrationRepo{}, nil)
+
+	_, err := service.RegisterWithEmail(context.Background(), "user@example.com", "securepass1", "", "", "127.0.0.1", "", requestmeta.SessionAuditContext{})
+	if err == nil || err.Error() != "verification code is invalid or expired" {
+		t.Fatalf("expected verification code error instead of turnstile error, got %v", err)
+	}
+}
+
 func TestResolveSecurityVerificationMethod(t *testing.T) {
 	now := time.Now()
 	cases := []struct {
diff --git a/backend/internal/application/auth/service.go b/backend/internal/application/auth/service.go
@@ -43,16 +43,11 @@ type Service struct {
 	geoResolver          *geoip.Client
 	subscriptionResolver subscriptionResolver
 	providerHTTPClient   *http.Client
-	turnstileHTTPClient  httpDoer
 	logger               *zap.Logger
 	storeProvider        appstorage.Provider
 	auditWriter          auditWriter
 }
 
-type httpDoer interface {
-	Do(*http.Request) (*http.Response, error)
-}
-
 type subscriptionResolver interface {
 	GetCurrentSubscriptionSnapshot(
 		ctx context.Context,
@@ -80,14 +75,12 @@ func NewServiceWithRuntime(cfg *config.Runtime, repo repository.AuthRepository,
 		ssrfProtectionEnabled = snapshot.SSRFProtectionEnabled
 	}
 	providerHTTPClient := newAuthOutboundHTTPClient(env, ssrfProtectionEnabled)
-	turnstileHTTPClient := newAuthOutboundHTTPClient(env, ssrfProtectionEnabled)
 	return &Service{
-		cfg:                 cfg,
-		repo:                repo,
-		geoResolver:         geoResolver,
-		providerHTTPClient:  providerHTTPClient,
-		turnstileHTTPClient: turnstileHTTPClient,
-		storeProvider:       appstorage.NewRuntimeProvider(cfg, nil),
+		cfg:                cfg,
+		repo:               repo,
+		geoResolver:        geoResolver,
+		providerHTTPClient: providerHTTPClient,
+		storeProvider:      appstorage.NewRuntimeProvider(cfg, nil),
 	}
 }
 
diff --git a/backend/internal/application/auth/turnstile.go b/backend/internal/application/auth/turnstile.go
@@ -3,19 +3,25 @@ package auth
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"net/http"
 	"net/url"
 	"strings"
 
 	"github.com/DEEIX-AI/DEEIX-Chat/backend/internal/infra/config"
-	"go.uber.org/zap"
 )
 
 const (
-	turnstileSiteverifyEndpoint = "https://challenges.cloudflare.com/turnstile/v0/siteverify"
-	turnstileTokenMaxLength     = 2048
+	turnstileTokenMaxLength = 2048
+)
+
+var (
+	errTurnstileNotConfigured = errors.New("turnstile is not configured")
+	errTurnstileRequired      = errors.New("turnstile verification is required")
+	errTurnstileTokenTooLong  = errors.New("turnstile token is too long")
+	errTurnstileFailed        = errors.New("turnstile verification failed")
 )
 
 type turnstileSiteverifyResponse struct {
@@ -28,17 +34,20 @@ func (s *Service) verifyRegistrationTurnstile(ctx context.Context, cfg config.Co
 		return nil
 	}
 	siteKey := strings.TrimSpace(cfg.TurnstileSiteKey)
+	if siteKey == "" {
+		return nil
+	}
 	secretKey := strings.TrimSpace(cfg.TurnstileSecretKey)
-	if siteKey == "" || secretKey == "" {
-		return fmt.Errorf("turnstile is not configured")
+	if secretKey == "" {
+		return errTurnstileNotConfigured
 	}
 
 	token := strings.TrimSpace(tokenValue)
 	if token == "" {
-		return fmt.Errorf("turnstile verification is required")
+		return errTurnstileRequired
 	}
 	if len(token) > turnstileTokenMaxLength {
-		return fmt.Errorf("turnstile token is too long")
+		return errTurnstileTokenTooLong
 	}
 
 	form := url.Values{}
@@ -48,33 +57,32 @@ func (s *Service) verifyRegistrationTurnstile(ctx context.Context, cfg config.Co
 		form.Set("remoteip", ip)
 	}
 
-	request, err := http.NewRequestWithContext(ctx, http.MethodPost, turnstileSiteverifyEndpoint, strings.NewReader(form.Encode()))
+	endpoint := strings.TrimSpace(cfg.TurnstileSiteverifyURL)
+	if endpoint == "" {
+		endpoint = config.DefaultTurnstileSiteverifyURL
+	}
+	request, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint, strings.NewReader(form.Encode()))
 	if err != nil {
 		return err
 	}
 	request.Header.Set("Content-Type", "application/x-www-form-urlencoded")
 	request.Header.Set("Accept", "application/json")
 
-	client := s.turnstileHTTPClient
-	if client == nil {
-		s.warn("turnstile_siteverify_client_missing")
-		return fmt.Errorf("turnstile verification failed")
-	}
-	response, err := client.Do(request)
+	response, err := s.providerHTTPClient.Do(request)
 	if err != nil {
-		s.warn("turnstile_siteverify_request_failed", zap.Error(err))
-		return fmt.Errorf("turnstile verification failed")
+		s.warn("turnstile siteverify request failed: " + err.Error())
+		return errTurnstileFailed
 	}
 	defer response.Body.Close()
 
 	var result turnstileSiteverifyResponse
 	if err = json.NewDecoder(io.LimitReader(response.Body, 1<<20)).Decode(&result); err != nil {
-		s.warn("turnstile_siteverify_decode_failed", zap.Int("status", response.StatusCode), zap.Error(err))
-		return fmt.Errorf("turnstile verification failed")
+		s.warn(fmt.Sprintf("turnstile siteverify decode failed: status=%d error=%v", response.StatusCode, err))
+		return errTurnstileFailed
 	}
 	if response.StatusCode < http.StatusOK || response.StatusCode >= http.StatusMultipleChoices || !result.Success {
-		s.warn("turnstile_siteverify_rejected", zap.Int("status", response.StatusCode), zap.Strings("error_codes", result.ErrorCodes))
-		return fmt.Errorf("turnstile verification failed")
+		s.warn(fmt.Sprintf("turnstile siteverify rejected: status=%d error_codes=%s", response.StatusCode, strings.Join(result.ErrorCodes, ",")))
+		return errTurnstileFailed
 	}
 	return nil
 }
diff --git a/backend/internal/infra/config/config.go b/backend/internal/infra/config/config.go
@@ -26,6 +26,9 @@ const (
 )
 
 const (
+	// DefaultTurnstileSiteverifyURL 是 Cloudflare Turnstile 默认校验端点。
+	DefaultTurnstileSiteverifyURL = "https://challenges.cloudflare.com/turnstile/v0/siteverify"
+
 	// DefaultMCPMaxSelectedToolsPerMessage 是单次消息可选择 MCP 工具数量的默认值。
 	DefaultMCPMaxSelectedToolsPerMessage = 32
 	// MaxMCPSelectedToolsPerMessage 是运行时配置允许的安全上限，防止一次请求暴露过多工具 schema。
@@ -204,9 +207,10 @@ type yamlConfig struct {
 		MaxHeaderBytes           int    `yaml:"max_header_bytes"`
 	} `yaml:"server"`
 	Security struct {
-		JWTSecret             string `yaml:"jwt_secret"`
-		DataEncryptionKey     string `yaml:"data_encryption_key"`
-		SSRFProtectionEnabled *bool  `yaml:"ssrf_protection_enabled"`
+		JWTSecret              string `yaml:"jwt_secret"`
+		DataEncryptionKey      string `yaml:"data_encryption_key"`
+		SSRFProtectionEnabled  *bool  `yaml:"ssrf_protection_enabled"`
+		TurnstileSiteverifyURL string `yaml:"turnstile_siteverify_url"`
 	} `yaml:"security"`
 	Database struct {
 		Postgres struct {
@@ -309,6 +313,7 @@ type Config struct {
 	SMTPUsername                 string
 	SMTPPassword                 string
 	SMTPFrom                     string
+	TurnstileSiteverifyURL       string
 	OTelEnabled                  *bool
 	OTelExporterOTLPEndpoint     string
 	OTelExporterOTLPHeaders      string
@@ -512,6 +517,7 @@ func Load() Config {
 		SMTPUsername:                 "",
 		SMTPPassword:                 "",
 		SMTPFrom:                     "",
+		TurnstileSiteverifyURL:       envOr("TURNSTILE_SITEVERIFY_URL", yc.Security.TurnstileSiteverifyURL, DefaultTurnstileSiteverifyURL),
 		OTelEnabled:                  envOrBoolOptional("OTEL_ENABLED", yc.Observability.Tracing.Enabled),
 		OTelExporterOTLPEndpoint:     envOr("OTEL_EXPORTER_OTLP_ENDPOINT", yc.Observability.Tracing.Endpoint, ""),
 		OTelExporterOTLPHeaders:      envOr("OTEL_EXPORTER_OTLP_HEADERS", yc.Observability.Tracing.Headers, ""),
diff --git a/backend/internal/infra/config/config_test.go b/backend/internal/infra/config/config_test.go
@@ -97,6 +97,31 @@ geoip:
 	assertPath(t, "geoip database", cfg.GeoIPDatabasePath, filepath.Join(root, "data", "geoip.mmdb"))
 }
 
+func TestLoadReadsTurnstileSiteverifyURL(t *testing.T) {
+	cleanupConfigEnv(t)
+
+	configPath := filepath.Join(t.TempDir(), "config.yaml")
+	configBody := []byte(`
+security:
+  turnstile_siteverify_url: "https://turnstile.example.test/siteverify"
+`)
+	if err := os.WriteFile(configPath, configBody, 0o644); err != nil {
+		t.Fatalf("write config: %v", err)
+	}
+	t.Setenv("CONFIG_FILE", configPath)
+
+	cfg := Load()
+	if cfg.TurnstileSiteverifyURL != "https://turnstile.example.test/siteverify" {
+		t.Fatalf("expected turnstile siteverify url from config, got %q", cfg.TurnstileSiteverifyURL)
+	}
+
+	t.Setenv("TURNSTILE_SITEVERIFY_URL", "https://turnstile-env.example.test/siteverify")
+	cfg = Load()
+	if cfg.TurnstileSiteverifyURL != "https://turnstile-env.example.test/siteverify" {
+		t.Fatalf("expected turnstile siteverify url from env, got %q", cfg.TurnstileSiteverifyURL)
+	}
+}
+
 func TestValidateAllowsOnlyDevAndProdEnvironment(t *testing.T) {
 	tests := []struct {
 		name    string
@@ -145,6 +170,7 @@ func cleanupConfigEnv(t *testing.T) {
 		"FRONTEND_DIST_DIR",
 		"STORAGE_ROOT_DIR",
 		"GEOIP_DATABASE_PATH",
+		"TURNSTILE_SITEVERIFY_URL",
 	}
 	for _, key := range keys {
 		key := key
diff --git a/backend/internal/infra/llm/openai_images.go b/backend/internal/infra/llm/openai_images.go
@@ -471,7 +471,7 @@ func applyOpenAIImageEditStreamParams(fields map[string]string, options map[stri
 	if !ok {
 		return
 	}
-	if value >= 0 {
+	if value > 0 {
 		fields["partial_images"] = fmt.Sprintf("%d", value)
 	}
 }
@@ -554,7 +554,7 @@ func applyOpenAIImageGenerationStreamParams(payload map[string]interface{}, opti
 	if !ok {
 		return
 	}
-	if value >= 0 {
+	if value > 0 {
 		payload["partial_images"] = value
 	}
 }
diff --git a/backend/internal/infra/llm/openai_images_test.go b/backend/internal/infra/llm/openai_images_test.go
@@ -80,6 +80,19 @@ func TestBuildOpenAIImageGenerationStreamRequestBodyOmitsDefaultPartialImages(t
 	}
 }
 
+func TestBuildOpenAIImageGenerationStreamRequestBodyOmitsZeroPartialImages(t *testing.T) {
+	payload, err := buildOpenAIImageGenerationStreamRequestBody("gpt-image-1", GenerateInput{
+		Messages: []Message{{Role: "user", Content: "A clean product render"}},
+		Options:  map[string]interface{}{"partial_images": 0},
+	})
+	if err != nil {
+		t.Fatalf("build image stream request body: %v", err)
+	}
+	if _, ok := payload["partial_images"]; ok {
+		t.Fatalf("partial_images must only be sent when configured as a positive value, got %#v", payload)
+	}
+}
+
 func TestBuildOpenAIImageGenerationRequestBodyDallEParams(t *testing.T) {
 	payload, err := buildOpenAIImageGenerationRequestBody("dall-e-3", GenerateInput{
 		Messages: []Message{{Role: "user", Content: "A clean product render"}},
@@ -183,6 +196,30 @@ func TestBuildOpenAIImageEditStreamMultipartRequest(t *testing.T) {
 	}
 }
 
+func TestBuildOpenAIImageEditStreamMultipartRequestOmitsZeroPartialImages(t *testing.T) {
+	body, contentType, _, err := buildOpenAIImageEditMultipartRequest("gpt-image-1", GenerateInput{
+		Messages: []Message{{
+			Role: "user",
+			Parts: []ContentPart{
+				{Kind: ContentPartText, Text: "Replace the background"},
+				{Kind: ContentPartImage, MimeType: "image/png", Data: []byte("source")},
+			},
+		}},
+		Options: map[string]interface{}{"partial_images": 0},
+	}, true)
+	if err != nil {
+		t.Fatalf("build image edit stream multipart request: %v", err)
+	}
+	req := httptest.NewRequest(http.MethodPost, "/v1/images/edits", bytes.NewReader(body))
+	req.Header.Set("Content-Type", contentType)
+	if err = req.ParseMultipartForm(10 << 20); err != nil {
+		t.Fatalf("parse multipart body: %v", err)
+	}
+	if _, ok := req.MultipartForm.Value["partial_images"]; ok {
+		t.Fatalf("partial_images must only be sent when configured as a positive value, got %#v", req.MultipartForm.Value)
+	}
+}
+
 func TestParseOpenAIImageGenerationOutput(t *testing.T) {
 	output, err := parseOpenAIImageOutput([]byte(`{
 		"created": 1713833628,
diff --git a/config.docker.example.yaml b/config.docker.example.yaml
diff --git a/config.example.yaml b/config.example.yaml

Original file line number	Diff line number	Diff line change
`@@ -161,6 +161,8 @@ func (s *Service) RegisterWithEmail(ctx context.Context, email string, password`
`161`	`161`	`if err != nil {`
`162`	`162`	`return nil, err`
`163`	`163`	`}`
	`164`	`+ // With email verification enabled, Turnstile is checked before issuing the registration code.`
	`165`	`+ // Without that step, completion is the first registration write path and must verify it here.`
`164`	`166`	`if !cfg.EmailVerificationEnabled {`
`165`	`167`	`if err = s.verifyRegistrationTurnstile(ctx, cfg, turnstileToken, remoteIP); err != nil {`
`166`	`168`	`return nil, err`
Original file line number	Diff line number	Diff line change
`@@ -471,7 +471,7 @@ func applyOpenAIImageEditStreamParams(fields map[string]string, options map[stri`
`471`	`471`	`if !ok {`
`472`	`472`	`return`
`473`	`473`	`}`
`474`		`- if value >= 0 {`
	`474`	`+ if value > 0 {`
`475`	`475`	`fields["partial_images"] = fmt.Sprintf("%d", value)`
`476`	`476`	`}`
`477`	`477`	`}`
`@@ -554,7 +554,7 @@ func applyOpenAIImageGenerationStreamParams(payload map[string]interface{}, opti`
`554`	`554`	`if !ok {`
`555`	`555`	`return`
`556`	`556`	`}`
`557`		`- if value >= 0 {`
	`557`	`+ if value > 0 {`
`558`	`558`	`payload["partial_images"] = value`
`559`	`559`	`}`
`560`	`560`	`}`