Enhance feature flag evaluation and configuration handling
Some checks failed
DB migration compat / Check if migrations changed (push) Has been cancelled
DB migration compat / Back-compat — Current branch migrations with ${{ needs.check-migrations-changed.outputs.base_branch }} branch code (push) Has been cancelled
DB migration compat / Forward-compat — Current branch code with ${{ needs.check-migrations-changed.outputs.base_branch }} branch migrations (push) Has been cancelled
DB migration compat / No migration changes (skipped) (push) Has been cancelled

- Introduced verification for primary email in the feature flag evaluation context to ensure only verified emails are used.
- Updated the evaluation logic to handle distinct IDs more flexibly, allowing for better fallback mechanisms.
- Improved the PATCH route for configuration overrides by implementing a new method to merge old and new configurations, ensuring validation before writing.
- Added comprehensive tests to validate the new feature flag evaluation scenarios, including handling of unverified emails and dependency cycles.
This commit is contained in:
mantrakp04 2026-04-30 15:57:31 -07:00
parent a6ff394f7c
commit 4157732054
11 changed files with 336 additions and 20 deletions

View File

@ -49,6 +49,7 @@ export const POST = createSmartRouteHandler({
const config: FeatureFlagsConfig = auth.tenancy.config.featureFlags;
const callerCanSupplyTargetingContext = auth.type !== "client";
const verifiedPrimaryEmail = auth.user?.primary_email_verified ? auth.user.primary_email ?? undefined : undefined;
const evalContext: EvalContext = callerCanSupplyTargetingContext ? {
distinctId: body.distinct_id ?? body.user_id ?? auth.user?.id,
userId: body.user_id ?? auth.user?.id,
@ -58,13 +59,13 @@ export const POST = createSmartRouteHandler({
context: body.context,
cohorts: body.cohorts,
} : {
distinctId: auth.user?.id,
distinctId: body.distinct_id ?? auth.user?.id,
userId: auth.user?.id,
user: auth.user ? {
id: auth.user.id,
primary_email: auth.user.primary_email,
primary_email: verifiedPrimaryEmail,
primary_email_verified: auth.user.primary_email_verified,
email: auth.user.primary_email,
email: verifiedPrimaryEmail,
} : undefined,
};

View File

@ -18,6 +18,7 @@ import { LOCAL_EMULATOR_ENV_CONFIG_BLOCKED_MESSAGE, isLocalEmulatorProject } fro
import { globalPrismaClient, rawQuery } from "@/prisma-client";
import { createSmartRouteHandler } from "@/route-handlers/smart-route-handler";
import { branchConfigSchema, environmentConfigSchema, getConfigOverrideErrors, migrateConfigOverride, projectConfigSchema } from "@stackframe/stack-shared/dist/config/schema";
import { override } from "@stackframe/stack-shared/dist/config/format";
import { adaptSchema, branchConfigSourceSchema, serverOrHigherAuthTypeSchema, yupNumber, yupObject, yupString } from "@stackframe/stack-shared/dist/schema-fields";
import { StackAssertionError, StatusError, captureError } from "@stackframe/stack-shared/dist/utils/errors";
import * as yup from "yup";
@ -317,10 +318,22 @@ export const PATCH = createSmartRouteHandler({
const levelConfig = levelConfigs[req.params.level];
const parsedConfig = await parseAndValidateConfig(req.body.config_override_string, levelConfig);
const newConfig = await levelConfig.override({
const oldConfig = await levelConfig.get({
projectId: req.auth.tenancy.project.id,
branchId: req.auth.tenancy.branchId,
config: parsedConfig,
});
const newConfig = override(oldConfig, parsedConfig);
await assertConfigValidBeforeWrite(levelConfig, {
projectId: req.auth.tenancy.project.id,
branchId: req.auth.tenancy.branchId,
config: newConfig,
});
await levelConfig.set({
projectId: req.auth.tenancy.project.id,
branchId: req.auth.tenancy.branchId,
config: newConfig,
});
await warnOnValidationFailure(levelConfig, {

View File

@ -292,11 +292,34 @@ export default function PageClient() {
// Live evaluation against current config + the JSON-edited debug context.
const debugResult = (() => {
const parsedVariants = tryParseJson(variantsJson);
if (!parsedVariants.ok) return { error: `Invalid variants JSON: ${parsedVariants.error}` };
const variants = validateVariants(parsedVariants.value);
if (!variants.ok) return { error: variants.error };
const parsedRules = tryParseJson(rulesJson);
if (!parsedRules.ok) return { error: `Invalid rules JSON: ${parsedRules.error}` };
const rules = validateRules(parsedRules.value);
if (!rules.ok) return { error: rules.error };
const parsed = tryParseJson(debugContextJson);
if (!parsed.ok) return { error: parsed.error };
const ctx = validateEvalContext(parsed.value);
if (!ctx.ok) return { error: ctx.error };
const cfg: FeatureFlagsConfig = config.featureFlags;
const cfg: FeatureFlagsConfig = {
...config.featureFlags,
flags: {
...config.featureFlags.flags,
[flagId]: {
...flag,
description: description.trim() || undefined,
defaultVariantKey: defaultVariantKey.trim() || undefined,
variants: variants.value,
rules: rules.value,
},
},
};
return { result: evaluateFlag(flagId, cfg, ctx.value) };
})();

View File

@ -1,7 +1,8 @@
import { it } from "../../../../helpers";
import { evaluateFlag } from "@stackframe/stack-shared/dist/feature-flags/evaluator";
import type { FeatureFlagsConfig } from "@stackframe/stack-shared/dist/feature-flags/types";
import { Auth, Project, niceBackendFetch } from "../../../backend-helpers";
import { Auth, Project, backendContext, createMailbox, niceBackendFetch } from "../../../backend-helpers";
import { randomUUID } from "node:crypto";
async function setupProjectWithFlags(featureFlags: FeatureFlagsConfig) {
await Project.createAndSwitch();
@ -229,8 +230,48 @@ it("/feature-flags/evaluate scopes to flag_keys when provided", async ({ expect
expect(Object.keys(response.body.results)).toEqual(["f1"]);
});
it("/feature-flags/evaluate falls back to the authenticated user id when distinct_id is omitted", async ({ expect }) => {
it("/internal/config/override PATCH rejects feature flag updates that make the merged config invalid", async ({ expect }) => {
await setupProjectWithFlags({
flags: {
"flag-a": { key: "a", type: "boolean", enabled: true, defaultVariantKey: "off",
variants: { off: { value: false } }, rules: {} },
"flag-b": { key: "b", type: "boolean", enabled: true, defaultVariantKey: "off",
variants: { off: { value: false } }, rules: {} },
},
});
const response = await niceBackendFetch("/api/latest/internal/config/override/branch", {
method: "PATCH",
accessType: "admin",
body: {
config_override_string: JSON.stringify({
"featureFlags.flags.flag-b": {
key: "a",
type: "boolean",
enabled: true,
defaultVariantKey: "off",
variants: { off: { value: false } },
rules: {},
},
}),
},
});
expect(response.status).toBe(400);
const stillValidResponse = await niceBackendFetch("/api/latest/feature-flags/evaluate", {
method: "POST",
accessType: "client",
body: { flag_keys: ["b"] },
});
expect(stillValidResponse.body.results.b).toMatchObject({
flag_key: "b",
variant_key: "off",
value: false,
});
});
it("/feature-flags/evaluate uses client distinct_id when provided and auth user id when omitted", async ({ expect }) => {
const featureFlags: FeatureFlagsConfig = {
flags: {
"rollout-flag": {
key: "rollout",
@ -246,7 +287,8 @@ it("/feature-flags/evaluate falls back to the authenticated user id when distinc
},
},
},
});
};
await setupProjectWithFlags(featureFlags);
const { userId } = await Auth.Anonymous.signUp();
const omittedDistinctId = await niceBackendFetch("/api/latest/feature-flags/evaluate", {
@ -257,13 +299,20 @@ it("/feature-flags/evaluate falls back to the authenticated user id when distinc
const explicitUserId = await niceBackendFetch("/api/latest/feature-flags/evaluate", {
method: "POST",
accessType: "client",
body: { distinct_id: "caller-supplied-id-is-ignored", flag_keys: ["rollout"] },
body: { distinct_id: "caller-supplied-id-is-used", flag_keys: ["rollout"] },
});
const omittedExpected = evaluateFlag("rollout-flag", featureFlags, { distinctId: userId, userId });
const explicitExpected = evaluateFlag("rollout-flag", featureFlags, { distinctId: "caller-supplied-id-is-used", userId });
expect(omittedDistinctId.body.results.rollout).toMatchObject({
flag_key: "rollout",
variant_key: explicitUserId.body.results.rollout.variant_key,
value: explicitUserId.body.results.rollout.value,
variant_key: omittedExpected.variantKey,
value: omittedExpected.value,
});
expect(explicitUserId.body.results.rollout).toMatchObject({
flag_key: "rollout",
variant_key: explicitExpected.variantKey,
value: explicitExpected.value,
});
});
@ -323,6 +372,47 @@ it("/feature-flags/evaluate ignores caller-supplied targeting attributes for cli
});
});
it("/feature-flags/evaluate does not expose unverified primary email as trusted client targeting context", async ({ expect }) => {
await setupProjectWithFlags({
flags: {
"internal-tools": {
key: "internal-tools",
type: "boolean",
enabled: true,
defaultVariantKey: "off",
variants: { on: { value: true }, off: { value: false } },
rules: {
employee: {
priority: 0,
enabled: true,
rolloutPercentage: 100,
variantKey: "on",
conditions: {
email: { attribute: "user.email", operator: "regex", value: "@stack-auth\\.com$" },
},
},
},
},
},
});
backendContext.set({ mailbox: createMailbox(`unverified-feature-flag-user--${randomUUID()}@stack-auth.com`) });
await Auth.Password.signUpWithEmail({ noWaitForEmail: true });
const response = await niceBackendFetch("/api/latest/feature-flags/evaluate", {
method: "POST",
accessType: "client",
body: {
flag_keys: ["internal-tools"],
},
});
expect(response.body.results["internal-tools"]).toMatchObject({
variant_key: "off",
value: false,
reason: "default",
});
});
it("/feature-flags/evaluate returns missing for requested unknown flag keys", async ({ expect }) => {
await setupProjectWithFlags({
flags: {

View File

@ -392,3 +392,6 @@ A: Use a strict root `postinstall` script that rewrites only Next `>=16` app-pag
Q: Why can Turbo-pruned Docker builds fail with `Cannot find module /app/scripts/postinstall-patch-next-async-debug-info.mjs` during `pnpm install`?
A: In pruned builder stages, we copy `/app/out/json` and run `pnpm install` before copying `/app/out/full`. The root `package.json` still runs `postinstall: node ./scripts/postinstall-patch-next-async-debug-info.mjs`, but that script is not present yet. Fix by copying `scripts/postinstall-patch-next-async-debug-info.mjs` into the builder stage before `pnpm install` (for all Dockerfiles using the prune pattern).
Q: Why can feature flag E2E tests fail with `The key "featureFlags" is not valid` on a branch that adds the schema?
A: The backend routes import generated package output from `@stackframe/stack-shared/dist`. If the dev package generator has not rebuilt that dist after adding `featureFlags` to the schema/app ids, backend E2E setup calls like `Project.updatePushedConfig({ featureFlags: ... })` still run against the old schema and reject the key before the test logic starts.

View File

@ -16,14 +16,25 @@ const apiUrl = process.env.NEXT_PUBLIC_STACK_API_URL;
const projectId = process.env.NEXT_PUBLIC_STACK_PROJECT_ID;
const publishableClientKey = process.env.NEXT_PUBLIC_STACK_PUBLISHABLE_CLIENT_KEY;
function getRequiredPublicEnv(name: string, value: string | undefined): string {
if (value == null || value.length === 0) {
throw new Error(`Expected ${name} to be configured for the feature flags demo`);
}
return value;
}
function getDemoApiUrl(): string {
return getRequiredPublicEnv("NEXT_PUBLIC_STACK_API_URL", apiUrl).replace(/\/+$/, "");
}
async function evaluateFlags(body: Record<string, unknown>): Promise<Record<string, EvalResult>> {
const res = await fetch(`${apiUrl}/api/latest/feature-flags/evaluate`, {
const res = await fetch(`${getDemoApiUrl()}/api/latest/feature-flags/evaluate`, {
method: "POST",
headers: {
"content-type": "application/json",
"x-stack-access-type": "client",
"x-stack-project-id": projectId ?? "",
"x-stack-publishable-client-key": publishableClientKey ?? "",
"x-stack-project-id": getRequiredPublicEnv("NEXT_PUBLIC_STACK_PROJECT_ID", projectId),
"x-stack-publishable-client-key": getRequiredPublicEnv("NEXT_PUBLIC_STACK_PUBLISHABLE_CLIENT_KEY", publishableClientKey),
},
body: JSON.stringify(body),
});

View File

@ -205,7 +205,7 @@ const branchSchemaFuzzerConfig = [{
"some-flag-id": [{
key: ["new-checkout"],
description: ["A flag for the new checkout"],
type: ["boolean", "multivariate", "json", "numeric", "string"] as const,
type: ["boolean"] as const,
enabled: [true, false],
killSwitch: [true, false],
tags: [{ "beta": [true], "internal": [true] }] as const,
@ -250,6 +250,24 @@ const branchSchemaFuzzerConfig = [{
}],
}],
}],
"another-flag-id": [{
key: ["dependency-flag"],
description: [undefined],
type: ["boolean"] as const,
enabled: [true],
killSwitch: [false],
tags: [undefined],
ownerUserId: [undefined],
dependsOn: [undefined],
holdoutId: [undefined],
defaultVariantKey: ["on"],
variants: [{
"on": [{
value: [true],
}],
}],
rules: [{}],
}],
}],
holdouts: [{
"some-holdout-id": [{

View File

@ -218,10 +218,14 @@ const featureFlagStickyBy = ["userId", "teamId", "distinctId"] as const;
const featureFlagTypes = ["boolean", "multivariate", "json", "numeric", "string"] as const;
type FeatureFlagSchemaValue = {
holdouts?: Record<string, unknown>,
flags?: Record<string, {
key: string,
type?: string,
variants?: Record<string, unknown>,
defaultVariantKey?: string,
dependsOn?: string,
holdoutId?: string,
rules?: Record<string, {
variantKey?: string,
variantWeights?: Record<string, unknown>,
@ -229,6 +233,43 @@ type FeatureFlagSchemaValue = {
}>,
};
function isJsonCompatibleFeatureFlagValue(value: unknown): boolean {
if (value === null) return true;
if (typeof value === "string" || typeof value === "boolean") return true;
if (typeof value === "number") return Number.isFinite(value);
if (Array.isArray(value)) {
return value.every((item) => item !== undefined && isJsonCompatibleFeatureFlagValue(item));
}
if (value !== undefined && typeof value === "object") {
return Object.values(value).every((nestedValue) => nestedValue !== undefined && isJsonCompatibleFeatureFlagValue(nestedValue));
}
return false;
}
function getFeatureFlagVariantValueError(flagType: string | undefined, variantValue: unknown): string | undefined {
switch (flagType) {
case "boolean": {
return typeof variantValue === "boolean" ? undefined : "boolean flag variants must have boolean values";
}
case "numeric": {
return typeof variantValue === "number" && Number.isFinite(variantValue) ? undefined : "numeric flag variants must have finite number values";
}
case "string": {
return typeof variantValue === "string" ? undefined : "string flag variants must have string values";
}
case "json": {
return isJsonCompatibleFeatureFlagValue(variantValue) ? undefined : "json flag variants must have JSON-compatible values";
}
case "multivariate":
case undefined: {
return undefined;
}
default: {
return `unknown flag type "${flagType}"`;
}
}
}
export const branchFeatureFlagsSchema = yupObject({
flags: yupRecord(
userSpecifiedIdSchema("flagId"),
@ -356,6 +397,8 @@ export const branchFeatureFlagsSchema = yupObject({
'Feature flag variant references must point to existing variants',
function(this: yup.TestContext<yup.AnyObject>, value: FeatureFlagSchemaValue | undefined) {
if (!value?.flags) return true;
const flagIds = new Set(Object.keys(value.flags));
const holdoutIds = new Set(Object.keys(value.holdouts ?? {}));
for (const [flagId, flag] of Object.entries(value.flags)) {
const variants = flag.variants ?? {};
const variantKeys = new Set(Object.keys(variants));
@ -365,6 +408,28 @@ export const branchFeatureFlagsSchema = yupObject({
path: `${this.path}.flags.${flagId}.defaultVariantKey`,
});
}
if (flag.dependsOn !== undefined && !flagIds.has(flag.dependsOn)) {
return this.createError({
message: `Feature flag "${flagId}" dependsOn "${flag.dependsOn}" does not reference an existing flag`,
path: `${this.path}.flags.${flagId}.dependsOn`,
});
}
if (flag.holdoutId !== undefined && !holdoutIds.has(flag.holdoutId)) {
return this.createError({
message: `Feature flag "${flagId}" holdoutId "${flag.holdoutId}" does not reference an existing holdout`,
path: `${this.path}.flags.${flagId}.holdoutId`,
});
}
for (const [variantKey, variant] of Object.entries(variants)) {
const variantValue = variant != null && typeof variant === "object" && !Array.isArray(variant) ? (variant as { value?: unknown }).value : undefined;
const variantValueError = getFeatureFlagVariantValueError(flag.type, variantValue);
if (variantValueError !== undefined) {
return this.createError({
message: `Feature flag "${flagId}" variant "${variantKey}" is invalid: ${variantValueError}`,
path: `${this.path}.flags.${flagId}.variants.${variantKey}.value`,
});
}
}
const rules = flag.rules ?? {};
for (const [ruleId, rule] of Object.entries(rules)) {
if (rule.variantKey !== undefined && !variantKeys.has(rule.variantKey)) {
@ -388,6 +453,52 @@ export const branchFeatureFlagsSchema = yupObject({
);
// --- END Feature Flags Schema ---
import.meta.vitest?.test("branchFeatureFlagsSchema rejects missing dependency and holdout references", async ({ expect }) => {
await expect(branchFeatureFlagsSchema.validate({
flags: {
child: {
key: "child",
type: "boolean",
dependsOn: "missing-gate",
holdoutId: "missing-holdout",
defaultVariantKey: "off",
variants: {
off: { value: false },
},
},
},
holdouts: {},
})).rejects.toThrow('Feature flag "child" dependsOn "missing-gate" does not reference an existing flag');
});
import.meta.vitest?.test("branchFeatureFlagsSchema validates variant values against flag type", async ({ expect }) => {
await expect(branchFeatureFlagsSchema.validate({
flags: {
flag: {
key: "flag",
type: "boolean",
defaultVariantKey: "on",
variants: {
on: { value: "true" },
},
},
},
})).rejects.toThrow('Feature flag "flag" variant "on" is invalid: boolean flag variants must have boolean values');
await expect(branchFeatureFlagsSchema.validate({
flags: {
flag: {
key: "flag",
type: "numeric",
defaultVariantKey: "one",
variants: {
one: { value: 1 },
},
},
},
})).resolves.toBeDefined();
});
export const branchConfigSchema = canNoLongerBeOverridden(projectConfigSchema, [
"sourceOfTruth",

View File

@ -175,7 +175,7 @@ function applyOperator(
}
case "in_cohort": {
if (typeof expected !== "string") return false;
return Boolean(cohorts?.[expected]);
return cohorts !== undefined && Object.prototype.hasOwnProperty.call(cohorts, expected) && cohorts[expected] === true;
}
}
}
@ -258,6 +258,16 @@ export function evaluateFlag(
context: EvalContext,
seenFlags: ReadonlySet<string> = new Set(),
): EvalResult {
if (seenFlags.has(flagKey)) {
const flag = config.flags?.[flagKey];
return {
flagKey,
variantKey: flag?.defaultVariantKey,
value: variantValue(flag ?? {}, flag?.defaultVariantKey),
reason: "cycle",
};
}
const flag = config.flags?.[flagKey];
if (!flag) {
return { flagKey, variantKey: undefined, value: undefined, reason: "missing" };
@ -275,8 +285,8 @@ export function evaluateFlag(
if (flag.enabled === false) return defaultResult("disabled");
if (flag.dependsOn) {
if (seenFlags.has(flag.dependsOn)) return defaultResult("cycle");
const dep = evaluateFlag(flag.dependsOn, config, context, new Set([...seenFlags, flagKey]));
if (dep.reason === "cycle") return defaultResult("cycle");
if (!isTruthyFlagValue(dep.value)) return defaultResult("dep_unmet");
}
@ -513,6 +523,21 @@ import.meta.vitest?.test("dependency cycles are broken safely", ({ expect }) =>
expect(r.value).toBe(false);
});
import.meta.vitest?.test("dependency cycles do not pass through truthy defaults", ({ expect }) => {
const config: FeatureFlagsConfig = {
flags: {
a: { key: "a", type: "boolean", enabled: true, defaultVariantKey: "on", dependsOn: "b",
variants: { on: { value: true }, off: { value: false } },
rules: { all: { variantKey: "off" } } },
b: { key: "b", type: "boolean", enabled: true, defaultVariantKey: "on", dependsOn: "a",
variants: { on: { value: true }, off: { value: false } } },
},
};
const r = evaluateFlag("a", config, { distinctId: "u" });
expect(r.reason).toBe("cycle");
expect(r.value).toBe(true);
});
import.meta.vitest?.test("holdout excludes a slice from any variant", ({ expect }) => {
const config: FeatureFlagsConfig = {
flags: {
@ -583,6 +608,22 @@ import.meta.vitest?.test("in_cohort matches cohort membership in context", ({ ex
expect(evaluateFlag("f", config, { distinctId: "u", cohorts: {} }).value).toBe(false);
});
import.meta.vitest?.test("in_cohort ignores inherited object properties", ({ expect }) => {
const config: FeatureFlagsConfig = {
flags: {
f: {
key: "f", type: "boolean", enabled: true, defaultVariantKey: "off",
variants: { on: { value: true }, off: { value: false } },
rules: { r: {
priority: 0, rolloutPercentage: 100, variantKey: "on",
conditions: { c: { attribute: "user.id", operator: "in_cohort", value: "toString" } },
} },
},
},
};
expect(evaluateFlag("f", config, { distinctId: "u", cohorts: {} }).value).toBe(false);
});
import.meta.vitest?.test("evaluation is sticky for the same distinctId", ({ expect }) => {
const config: FeatureFlagsConfig = {
flags: {

View File

@ -88,7 +88,9 @@ function createInterface(
projectId,
publishableClientKey: "pck_test",
});
vi.spyOn(iface, "evaluateFeatureFlags").mockImplementation(async (body) => await evaluateFeatureFlags(body));
Object.assign(iface, {
evaluateFeatureFlags: async (body: FeatureFlagEvaluateRequest) => await evaluateFeatureFlags(body),
});
return iface;
}
@ -177,6 +179,9 @@ describe("StackClientApp feature flag evaluation", () => {
await expect(app.getFeatureFlags(["present", "omitted"])).rejects.toThrow(
"Feature flag evaluate response did not include requested key omitted",
);
await expect(app.getFeatureFlags(["present", "toString"])).rejects.toThrow(
"Feature flag evaluate response did not include requested key toString",
);
});
it("evaluates feature flags through React-like hooks and caches equivalent ordered batches", async () => {

View File

@ -2731,7 +2731,7 @@ export class _StackClientAppImplIncomplete<HasTokenStore extends boolean, Projec
Object.entries(response.results).map(([key, result]) => [key, this._featureFlagResultFromCrud(result)]),
);
for (const key of new Set(keys)) {
if (!(key in results)) {
if (!Object.prototype.hasOwnProperty.call(results, key)) {
throwErr(`Feature flag evaluate response did not include requested key ${key}`);
}
}