fix: restore target_language validation on RFC 7396 PATCH

Switching the PATCH field to Optional[string] dropped the struct-tag
validation (no_null_bytes, max=35) — go-playground's validator can't reach
through the custom type — so PATCH accepted null bytes and over-long values
that PUT and the OpenAPI maxLength:35 still reject. Enforce the same bounds
in the PATCH value path (normalizeProvidedTargetLanguage), restoring
PUT/PATCH parity and honoring the contract.

Also pin settingKeyTargetLanguage to the EnrichmentSettings json tag with a
unit test, so a tag rename can't silently break PATCH null-removal.

ENG-1254

Author: xernobyl

internal/service/tenant_settings_service.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"strings"
+	"unicode/utf8"
 
 	"golang.org/x/text/language"
 
@@ -24,9 +25,15 @@ type TenantSettingsRepository interface {
 
 // settingKeyTargetLanguage is the JSONB key for the target language. It must match
 // the json tag on models.EnrichmentSettings.TargetLanguage; it is the key removed
-// when a PATCH sends target_language as null.
+// when a PATCH sends target_language as null. TestSettingKeyMatchesModelTag pins
+// this to the tag so a rename can't silently break null-removal.
 const settingKeyTargetLanguage = "target_language"
 
+// maxTargetLanguageLen bounds a provided target_language value. It mirrors the
+// `max=35` struct tag on UpdateTenantSettingsRequest (the PUT path) and the
+// OpenAPI maxLength, so PUT and PATCH enforce the same limit.
+const maxTargetLanguageLen = 35
+
 // TenantSettingsService reads and writes tenant-scoped enrichment settings. It is
 // the accessor enrichment workflows will use to resolve a tenant's configuration.
 type TenantSettingsService struct {
@@ -149,13 +156,25 @@ func normalizeTargetLanguage(raw string) (string, error) {
 // normalizeProvidedTargetLanguage validates a non-null target_language supplied in
 // a PATCH. Unlike PUT, an empty value is rejected: under RFC 7396 the way to
 // remove a setting is JSON null, so an explicit "" is a malformed locale rather
-// than a clear.
+// than a clear. It also enforces the same null-byte and length bounds the PUT path
+// gets from struct tags, which the Optional[string] field cannot carry.
 func normalizeProvidedTargetLanguage(raw string) (string, error) {
 	if strings.TrimSpace(raw) == "" {
 		return "", huberrors.NewValidationError(
 			"target_language",
 			"target_language must be a valid BCP-47 locale (e.g. en-US); send null to remove it")
 	}
 
+	if strings.ContainsRune(raw, '\x00') {
+		return "", huberrors.NewValidationError(
+			"target_language", "target_language must not contain null bytes")
+	}
+
+	if utf8.RuneCountInString(raw) > maxTargetLanguageLen {
+		return "", huberrors.NewValidationError(
+			"target_language",
+			fmt.Sprintf("target_language must be at most %d characters", maxTargetLanguageLen))
+	}
+
 	return normalizeTargetLanguage(raw)
 }

internal/service/tenant_settings_service_test.go

@@ -2,7 +2,9 @@ package service
 
 import (
 	"context"
+	"encoding/json"
 	"errors"
+	"strings"
 	"testing"
 
 	"github.com/formbricks/hub/internal/huberrors"
@@ -401,3 +403,48 @@ func TestTenantSettingsService_PatchSettings_RepoErrorPropagates(t *testing.T) {
 		t.Fatalf("PatchSettings() error = %v, want tenant write conflict", err)
 	}
 }
+
+func TestTenantSettingsService_PatchSettings_OversizedRejected(t *testing.T) {
+	repo := &mockTenantSettingsRepo{}
+	svc := NewTenantSettingsService(repo)
+
+	// Parity with PUT's max=35: a provided value longer than the bound is rejected
+	// before it reaches the repo (Optional[string] can't carry the struct tag).
+	_, err := svc.PatchSettings(context.Background(), "org-123", patchValue(strings.Repeat("a", maxTargetLanguageLen+1)))
+	if !errors.Is(err, huberrors.ErrValidation) {
+		t.Fatalf("PatchSettings() error = %v, want validation error for an oversized value", err)
+	}
+
+	if repo.patchCalled {
+		t.Fatal("repo.Patch called despite oversized value")
+	}
+}
+
+func TestTenantSettingsService_PatchSettings_NullByteRejected(t *testing.T) {
+	repo := &mockTenantSettingsRepo{}
+	svc := NewTenantSettingsService(repo)
+
+	_, err := svc.PatchSettings(context.Background(), "org-123", patchValue("en\x00US"))
+	if !errors.Is(err, huberrors.ErrValidation) {
+		t.Fatalf("PatchSettings() error = %v, want validation error for a null byte", err)
+	}
+
+	if repo.patchCalled {
+		t.Fatal("repo.Patch called despite null byte")
+	}
+}
+
+// TestSettingKeyMatchesModelTag pins settingKeyTargetLanguage to the json tag on
+// EnrichmentSettings.TargetLanguage, so a tag rename can't silently break PATCH
+// null-removal (which deletes by that key string).
+func TestSettingKeyMatchesModelTag(t *testing.T) {
+	raw, err := json.Marshal(models.EnrichmentSettings{TargetLanguage: "en-US"})
+	if err != nil {
+		t.Fatalf("marshal: %v", err)
+	}
+
+	if want := `"` + settingKeyTargetLanguage + `":`; !strings.Contains(string(raw), want) {
+		t.Fatalf("settingKeyTargetLanguage %q is not the json key in %s — const and tag have drifted",
+			settingKeyTargetLanguage, raw)
+	}
+}

tests/tenant_settings_test.go

@@ -346,6 +346,19 @@ func TestTenantSettings_PatchEmptyStringRejected(t *testing.T) {
 	assert.Equal(t, http.StatusBadRequest, resp.StatusCode)
 }
 
+// TestTenantSettings_PatchRejectsOversizedLanguage mirrors the PUT bound: a value
+// over the 35-char limit is rejected (the limit lives in the service for PATCH,
+// since Optional[string] cannot carry the struct tag PUT uses).
+func TestTenantSettings_PatchRejectsOversizedLanguage(t *testing.T) {
+	server, cleanup := setupTestServer(t)
+	defer cleanup()
+
+	body := `{"target_language":"` + strings.Repeat("a", 40) + `"}`
+	resp := settingsRequest(t, server.URL, http.MethodPatch, testTenantID("patch-oversized"), body, true)
+	require.NoError(t, resp.Body.Close())
+	assert.Equal(t, http.StatusBadRequest, resp.StatusCode)
+}
+
 // TestTenantSettings_PatchMergePreservesOtherKeys proves PATCH does a JSONB merge
 // (||), not a full replace: a key the typed model does not know about survives.
 func TestTenantSettings_PatchMergePreservesOtherKeys(t *testing.T) {