Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
5c91188
feat: add EncryptedJSONField backed by a rotatable Fernet key ring
Zaimwa9 Jul 14, 2026
770a97e
feat: add encrypted credentials column to warehouse connections
Zaimwa9 Jul 14, 2026
73640e9
feat: accept ClickHouse warehouse connection config and credentials
Zaimwa9 Jul 14, 2026
61a2a3a
feat: verify ClickHouse warehouse connections against the customer in…
Zaimwa9 Jul 14, 2026
9a05fe6
feat: wire ClickHouse verification into warehouse connection endpoints
Zaimwa9 Jul 14, 2026
6695381
refactor: extract warehouse validation into a dedicated module
Zaimwa9 Jul 15, 2026
98c0c76
test: neutral ClickHouse fixture data and bare Given/When/Then markers
Zaimwa9 Jul 15, 2026
81cf471
refactor: encrypt warehouse credentials with a SECRET_KEY-derived Fer…
Zaimwa9 Jul 15, 2026
f91b461
refactor: drop the ephemeral SECRET_KEY guard for warehouse credentials
Zaimwa9 Jul 16, 2026
b8c97cb
fix: reject internal network addresses for ClickHouse warehouse hosts
Zaimwa9 Jul 16, 2026
808513a
feat: throttle the warehouse test-connection endpoint
Zaimwa9 Jul 16, 2026
50891b5
feat: record why warehouse connection verification failed
Zaimwa9 Jul 16, 2026
171c14c
chore: update OpenAPI schema with warehouse connection fields
Zaimwa9 Jul 16, 2026
f7a302b
Merge remote-tracking branch 'origin/main' into feat/clickhouse-byo-w…
Zaimwa9 Jul 16, 2026
3589d6c
refactor: move the internal-address check from webhooks to core
Zaimwa9 Jul 16, 2026
77c343a
test: consolidate warehouse connection tests with parametrisation
Zaimwa9 Jul 16, 2026
87861f0
fix: harden warehouse connection validation, throttling, and verifica…
Zaimwa9 Jul 16, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions api/app/settings/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,7 @@
"invite": "10/min",
"user": USER_THROTTLE_RATE,
"influx_query": "5/min",
"warehouse_connection_write": "10/min",
},
"DEFAULT_FILTER_BACKENDS": ["django_filters.rest_framework.DjangoFilterBackend"],
"DEFAULT_RENDERER_CLASSES": [
Expand Down
1 change: 1 addition & 0 deletions api/app/settings/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
"user": "100000/day",
"master_api_key": "100000/day",
"influx_query": "50/min",
"warehouse_connection_write": "1000/min",
}

AWS_SSE_LOGS_BUCKET_NAME = "test_bucket"
Expand Down
45 changes: 45 additions & 0 deletions api/core/fields.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import base64
import hashlib
import json
from typing import Any

import structlog
from cryptography.fernet import Fernet, InvalidToken
from django.conf import settings
from django.db import models

logger = structlog.get_logger("core")


def _get_fernet() -> Fernet:
digest = hashlib.sha256(settings.SECRET_KEY.encode()).digest()
return Fernet(base64.urlsafe_b64encode(digest))
Comment on lines +14 to +16

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n## api/core/fields.py\n'
cat -n api/core/fields.py | sed -n '1,120p'

printf '\n## Search for _get_fernet and related encryption settings/tests\n'
rg -n "_get_fernet|Fernet|MultiFernet|SECRET_KEY|FIELD_ENCRYPTION|decrypt|from_db_value|Stored connection details are incomplete" api -S

printf '\n## api/core/services.py (relevant slices)\n'
if [ -f api/core/services.py ]; then
  cat -n api/core/services.py | sed -n '1,240p'
fi

printf '\n## tests mentioning SECRET_KEY rotation or from_db_value\n'
rg -n "secret_key changed|SECRET_KEY changed|from_db_value__secret_key_changed|returns none and logs|incomplete" tests api -S

Repository: Flagsmith/flagsmith

Length of output: 7390


🏁 Script executed:

#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path('api/core/fields.py')
print(p.exists(), p)
if p.exists():
    for i, line in enumerate(p.read_text().splitlines(), 1):
        if 1 <= i <= 80:
            print(f"{i:4d}: {line}")
PY

Repository: Flagsmith/flagsmith

Length of output: 1754


Use a dedicated key for encrypted fields. Deriving the Fernet key from SECRET_KEY couples credential encryption to Django’s global signing secret, and rotating SECRET_KEY will make stored values undecryptable while from_db_value returns None, which downstream surfaces as “Stored connection details are incomplete.” Add a separate encryption setting and a rotation path instead of reusing SECRET_KEY.



class EncryptedJSONField(models.TextField[Any, Any]):
def get_prep_value(self, value: Any) -> str | None:
if value is None:
return None
return _get_fernet().encrypt(json.dumps(value).encode()).decode()

def from_db_value(
self,
value: str | None,
expression: object,
connection: object,
) -> Any:
if value is None:
return None
try:
plaintext = _get_fernet().decrypt(value.encode())
except InvalidToken:
logger.warning("encrypted_field.decrypt_failed", exc_info=True)
return None
return json.loads(plaintext)

def get_lookup(self, lookup_name: str) -> Any:
if lookup_name != "isnull":
raise NotImplementedError(
"EncryptedJSONField only supports isnull lookups."
)
return super().get_lookup(lookup_name)
26 changes: 26 additions & 0 deletions api/core/network.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import ipaddress
import socket


def is_internal_address(hostname: str) -> bool:
"""Return True if the hostname is, or resolves to, an internal network
address: loopback, RFC 1918 private, link-local, reserved, or multicast.
Unresolvable hostnames are not considered internal."""
try:
ips = [ipaddress.ip_address(hostname)]
except ValueError:
# hostname is a name rather than a literal IP — resolve it.
try:
results = socket.getaddrinfo(hostname, None, socket.AF_UNSPEC)
ips = [ipaddress.ip_address(str(r[4][0]).split("%")[0]) for r in results]
except socket.gaierror:
return False
Comment on lines +9 to +17

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🔴 Critical | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf 'Files:\n'
git ls-files 'api/core/*' 'api/*webhooks*' 'api/*warehouse*' | sed -n '1,200p'

printf '\nOutline of api/core/network.py:\n'
ast-grep outline api/core/network.py --view expanded || true

printf '\nRelevant call sites for is_internal_address:\n'
rg -n "is_internal_address\(" api -S

printf '\nRead api/core/network.py with line numbers:\n'
cat -n api/core/network.py | sed -n '1,220p'

printf '\nRead the call-site files around matches (if any):\n'
for f in $(rg -l "is_internal_address\(" api -S || true); do
  printf '\n### %s ###\n' "$f"
  cat -n "$f" | sed -n '1,240p'
done

Repository: Flagsmith/flagsmith

Length of output: 20160


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf 'Search for validate_clickhouse_config / validate_credentials usage:\n'
rg -n "validate_clickhouse_config|validate_credentials|warehouse_validation" api -S

printf '\nRead experimentation serializers/models around the matches:\n'
for f in $(rg -l "validate_clickhouse_config|validate_credentials|warehouse_validation" api -S || true); do
  printf '\n### %s ###\n' "$f"
  cat -n "$f" | sed -n '1,260p'
done

Repository: Flagsmith/flagsmith

Length of output: 16963


Bound DNS resolution in this validator. socket.getaddrinfo has no timeout, so a hostile or broken resolver can hold this request path open for a long time. This runs synchronously during webhook URL and warehouse host validation, so a single hostname can tie up workers; use a bounded, fail-closed lookup strategy instead.


return any(
ip.is_loopback
or ip.is_private
or ip.is_link_local
or ip.is_reserved
or ip.is_multicast
for ip in ips
)
Comment on lines +19 to +26

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== api/core/network.py ==\n'
wc -l api/core/network.py
cat -n api/core/network.py

printf '\n== usages of the helper ==\n'
rg -n "is_loopback|is_private|is_link_local|is_reserved|is_multicast|ip\.ipv4_mapped|100\.64\.0\.0/10|shared address" api -S

printf '\n== python ipaddress probe ==\n'
python3 - <<'PY'
import ipaddress

tests = [
    "100.64.0.1",
    "100.127.255.254",
    "100.128.0.1",
    "::ffff:192.168.1.1",
    "::ffff:127.0.0.1",
    "::ffff:100.64.0.1",
    "192.168.1.1",
    "127.0.0.1",
    "169.254.1.1",
    "8.8.8.8",
]

for s in tests:
    ip = ipaddress.ip_address(s)
    attrs = {
        "is_global": ip.is_global,
        "is_private": ip.is_private,
        "is_loopback": ip.is_loopback,
        "is_link_local": ip.is_link_local,
        "is_reserved": ip.is_reserved,
        "is_multicast": ip.is_multicast,
    }
    if isinstance(ip, ipaddress.IPv6Address):
        attrs["ipv4_mapped"] = str(ip.ipv4_mapped) if ip.ipv4_mapped else None
    print(s, attrs)
PY

Repository: Flagsmith/flagsmith

Length of output: 3074


🌐 Web query:

Python ipaddress IPv4-mapped IPv6 is_private loopback behaviour shared address space 100.64.0.0/10 docs

💡 Result:

In Python's ipaddress module, the is_private and is_global attributes follow specific logic concerning the shared address space (100.64.0.0/10) and IPv4-mapped IPv6 addresses [1][2]. Shared Address Space (100.64.0.0/10) The 100.64.0.0/10 range is treated as a special case in the ipaddress module [1][2]. Both is_private and is_global return False for this network [2][3]. This is distinct from most other address ranges where is_private is the logical opposite of is_global [1][4]. IPv4-mapped IPv6 Addresses For IPv4-mapped IPv6 addresses (addresses starting with::FFFF/96), the is_private and is_global attributes are determined by the semantics of the underlying embedded IPv4 address [2][3]. The library enforces that the boolean value of the mapped address matches the value of the IPv4 address it contains [2]: address.is_private == address.ipv4_mapped.is_private address.is_global == address.ipv4_mapped.is_global This ensures consistency between the IPv6 representation and the underlying IPv4 address rules, including the special treatment of the 100.64.0.0/10 range [1][4]. General Behavior The is_private property is defined as True if an address is not globally reachable according to the IANA IPv4 or IPv6 special-purpose address registries, with the specified exceptions [1][5]. Conversely, is_global is True if the address is defined as globally reachable, again with the exception of the 100.64.0.0/10 range [2][3]. Developers should be aware that these properties reflect these IANA-based definitions rather than purely local or enterprise network configuration [5][6].

Citations:


Cover RFC 6598 shared address space

100.64.0.0/10 is neither is_private nor caught by the other predicates, so it still bypasses this internal-address check. Add an explicit shared-address-space check if these hosts should be treated as internal.

10 changes: 10 additions & 0 deletions api/experimentation/metrics.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import prometheus_client

flagsmith_experimentation_warehouse_connection_verifications_total = (
prometheus_client.Counter(
"flagsmith_experimentation_warehouse_connection_verifications_total",
"Outcomes of connection verification attempts against customers' own "
"data warehouses. `result` label is either `success` or `failure`.",
["result"],
)
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
from django.db import migrations

import core.fields


class Migration(migrations.Migration):
dependencies = [
("experimentation", "0009_add_rollout_segment"),
]

operations = [
migrations.AddField(
model_name="warehouseconnection",
name="credentials",
field=core.fields.EncryptedJSONField(blank=True, null=True),
),
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# Generated by Django 5.2.16 on 2026-07-16 09:01

from django.db import migrations, models


class Migration(migrations.Migration):
dependencies = [
("experimentation", "0010_warehouse_connection_credentials"),
]

operations = [
migrations.AddField(
model_name="warehouseconnection",
name="status_detail",
field=models.CharField(blank=True, max_length=255, null=True),
),
]
3 changes: 3 additions & 0 deletions api/experimentation/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
hook,
)

from core.fields import EncryptedJSONField
from core.models import SoftDeleteExportableModel
from environments.models import Environment
from experimentation.dataclasses import (
Expand Down Expand Up @@ -54,10 +55,12 @@ class WarehouseConnection(LifecycleModelMixin, SoftDeleteExportableModel): # ty
choices=WarehouseConnectionStatus.choices,
default=WarehouseConnectionStatus.CREATED,
)
status_detail = models.CharField(max_length=255, null=True, blank=True)
name = models.CharField(max_length=255)
config: models.JSONField[dict[str, object] | None, dict[str, object] | None] = (
models.JSONField(null=True, blank=True)
)
credentials = EncryptedJSONField(null=True, blank=True)
created_at = models.DateTimeField(auto_now_add=True)

# Populated at serialization time for flagsmith connections from ClickHouse;
Expand Down
45 changes: 24 additions & 21 deletions api/experimentation/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,17 @@
ExperimentStatus,
Metric,
WarehouseConnection,
WarehouseConnectionStatus,
WarehouseType,
)
from experimentation.services import (
apply_experiment_rollout,
get_experiment_rollout,
)
from experimentation.types import (
SNOWFLAKE_DEFAULTS,
MetricExperimentResult,
SnowflakeConfig,
from experimentation.types import MetricExperimentResult
from experimentation.warehouse_validation import (
CONFIG_VALIDATORS,
validate_credentials,
)
from features.feature_states.serializers import (
FeatureValueSerializer,
Expand All @@ -41,6 +42,9 @@
class WarehouseConnectionSerializer(serializers.ModelSerializer): # type: ignore[type-arg]
name = serializers.CharField(max_length=255, required=False)
config = serializers.JSONField(default=None, required=False, allow_null=True)
credentials = serializers.JSONField(
default=None, required=False, allow_null=True, write_only=True
)
total_events_received = serializers.SerializerMethodField()
unique_events_count = serializers.SerializerMethodField()

Expand All @@ -50,33 +54,45 @@ class Meta:
"id",
"warehouse_type",
"status",
"status_detail",
"name",
"config",
"credentials",
"created_at",
"total_events_received",
"unique_events_count",
)
read_only_fields = ("id", "status", "created_at")
read_only_fields = ("id", "status", "status_detail", "created_at")

def validate(self, attrs: dict[str, Any]) -> dict[str, Any]:
warehouse_type: str = attrs.get(
"warehouse_type",
getattr(self.instance, "warehouse_type", ""),
)
type_changed = (
self.instance is not None
and getattr(self.instance, "warehouse_type", "") != warehouse_type
)

if "config" not in attrs and self.instance is not None:
validate_credentials(attrs, warehouse_type, self.instance) # type: ignore[arg-type]

if "config" not in attrs and self.instance is not None and not type_changed:
return attrs

config: dict[str, Any] | None = attrs.get("config")

if warehouse_type == WarehouseType.SNOWFLAKE:
attrs["config"] = self._validate_snowflake_config(config or {})
if config_validator := CONFIG_VALIDATORS.get(warehouse_type):
attrs["config"] = config_validator(config or {})
elif warehouse_type == WarehouseType.FLAGSMITH:
if config:
raise serializers.ValidationError(
{"config": "Flagsmith warehouse does not accept configuration."}
)
attrs["config"] = None

if type_changed:
attrs["status"] = WarehouseConnectionStatus.CREATED
attrs["status_detail"] = None
return attrs

def create(
Expand Down Expand Up @@ -105,19 +121,6 @@ def _generate_name(warehouse_type: str, environment: Environment) -> str:
label = WarehouseType(warehouse_type).label
return f"{label} Warehouse - {environment.name}"

@staticmethod
def _validate_snowflake_config(config: dict[str, Any]) -> SnowflakeConfig:
account_identifier = config.get("account_identifier", "")
if not account_identifier:
raise serializers.ValidationError(
{"config": {"account_identifier": "This field is required."}}
)
merged: SnowflakeConfig = {
**SNOWFLAKE_DEFAULTS,
**config, # type: ignore[typeddict-item]
}
return merged


class MetricSerializer(serializers.ModelSerializer): # type: ignore[type-arg]
experiments = serializers.SerializerMethodField()
Expand Down
74 changes: 74 additions & 0 deletions api/experimentation/services.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import structlog
from clickhouse_driver import Client
from clickhouse_driver import errors as clickhouse_errors
from clickhouse_driver.util.helpers import parse_url
from django.conf import settings
from django.db import transaction
Expand All @@ -17,6 +18,7 @@
from audit.models import AuditLog
from audit.related_object_type import RelatedObjectType
from core.dataclasses import AuthorData
from core.network import is_internal_address
from environments.tasks import rebuild_environment_document
from experimentation.constants import (
CONTROL_VARIANT_KEY,
Expand All @@ -40,6 +42,9 @@
RolloutSpec,
WarehouseEventStats,
)
from experimentation.metrics import (
flagsmith_experimentation_warehouse_connection_verifications_total,
)
from experimentation.models import (
VALID_STATUS_TRANSITIONS,
Experiment,
Expand All @@ -56,6 +61,7 @@
compare_to_control,
srm_p_value,
)
from experimentation.types import ClickHouseConfig, ClickHouseCredentials
from features.models import FeatureState
from features.value_types import BOOLEAN, INTEGER, STRING
from features.versioning.dataclasses import FlagChangeSet, MultivariateValueChangeSet
Expand Down Expand Up @@ -88,6 +94,7 @@

CLICKHOUSE_CONNECT_TIMEOUT_SECONDS = 5
CLICKHOUSE_QUERY_TIMEOUT_SECONDS = 30
CLICKHOUSE_VERIFY_TIMEOUT_SECONDS = 5


def is_warehouse_feature_enabled(organisation: Organisation) -> bool:
Expand Down Expand Up @@ -787,6 +794,73 @@ def mark_warehouse_pending_connection(
return connection


class InternalAddressError(Exception):
pass


def _describe_verification_error(error: Exception) -> str:
if isinstance(error, clickhouse_errors.ServerException):
if error.code == 516:
return "Authentication failed."
if error.code == 81:
return "Database does not exist."
return "The ClickHouse server rejected the request."
if isinstance(error, (clickhouse_errors.SocketTimeoutError, TimeoutError)):
return "The connection timed out."
if isinstance(error, clickhouse_errors.NetworkError):
return "Could not connect to the host."
if isinstance(error, InternalAddressError):
return "Host must not target internal or private network addresses."
if isinstance(error, KeyError):
return "Stored connection details are incomplete."
return "Verification failed."


def verify_clickhouse_connection(connection: WarehouseConnection) -> None:
"""Run SELECT 1 against the customer's ClickHouse and set the status to
connected or errored; never raises."""
log = logger.bind(environment__id=connection.environment_id)
try:
log = log.bind(organisation__id=connection.environment.project.organisation_id)
config = typing.cast(ClickHouseConfig, connection.config or {})
credentials = typing.cast(ClickHouseCredentials, connection.credentials or {})
# Re-check right before connecting: DNS may resolve differently than it
# did at validation time, and rows may predate host validation.
if is_internal_address(config["host"]):
raise InternalAddressError(config["host"])
client = Client(
config["host"],
Comment on lines +827 to +832

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

Close the DNS-rebinding gap before opening the socket.

is_internal_address(config["host"]) and Client(config["host"], ...) resolve the hostname independently. An attacker-controlled hostname can return a public address for the check and an internal address for the client connection. Pin the validated address or enforce network-level egress denial for private/link-local destinations.

port=config["port"],
user=config["username"],
password=credentials["password"],
database=config["database"],
secure=config["secure"],
connect_timeout=CLICKHOUSE_CONNECT_TIMEOUT_SECONDS,
send_receive_timeout=CLICKHOUSE_VERIFY_TIMEOUT_SECONDS,
)
try:
client.execute("SELECT 1")
finally:
client.disconnect()
except Exception as error:
connection.status = WarehouseConnectionStatus.ERRORED
connection.status_detail = _describe_verification_error(error)
connection.save(update_fields=["status", "status_detail"])
flagsmith_experimentation_warehouse_connection_verifications_total.labels(
result="failure"
).inc()
log.warning("connection.verification_failed", exc_info=True)
return

connection.status = WarehouseConnectionStatus.CONNECTED
connection.status_detail = None
connection.save(update_fields=["status", "status_detail"])
flagsmith_experimentation_warehouse_connection_verifications_total.labels(
result="success"
).inc()
log.info("connection.verification_succeeded")
Comment on lines +819 to +861

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the verifier and its call sites.
printf '\n== services.py around verify_clickhouse_connection ==\n'
sed -n '780,900p' api/experimentation/services.py

printf '\n== views.py references ==\n'
rg -n "verify_clickhouse_connection|write_environment_ingestion_keys\.delay|AFTER_CREATE|perform_create|perform_update|test connection|connection.verification" api/experimentation -n

printf '\n== views.py relevant slices ==\n'
for f in api/experimentation/views.py api/experimentation/*.py; do
  if [ -f "$f" ] && rg -n "verify_clickhouse_connection|write_environment_ingestion_keys\.delay|perform_create|perform_update|AFTER_CREATE" "$f" >/dev/null; then
    echo "--- $f ---"
    sed -n '1,260p' "$f" | rg -n -A40 -B20 "verify_clickhouse_connection|write_environment_ingestion_keys\.delay|perform_create|perform_update|AFTER_CREATE"
  fi
done

Repository: Flagsmith/flagsmith

Length of output: 18677


Move ClickHouse verification off the request thread Up to ~35s of network I/O still runs inline in perform_create/perform_update, so a slow or unreachable ClickHouse host can block API workers on every create/update. Queue this asynchronously and update status/status_detail out of band.



def refresh_warehouse_connection_status(
connection: WarehouseConnection,
stats: WarehouseEventStats,
Expand Down
21 changes: 21 additions & 0 deletions api/experimentation/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,3 +44,24 @@ class SnowflakeConfig(TypedDict):
"role": "FLAGSMITH_LOADER",
"user": "FLAGSMITH_SERVICE",
}


class ClickHouseConfig(TypedDict):
host: str
port: int
database: str
username: str
secure: bool


CLICKHOUSE_DEFAULTS: ClickHouseConfig = {
"host": "",
"port": 9440,
"database": "flagsmith",
"username": "default",
"secure": True,
}


class ClickHouseCredentials(TypedDict):
password: str
Loading
Loading