Skip to content

Commit 20c8f9e

Browse files
committed
fix(webapp): stop an unreadable eval override reading as consent to judge
flag() ignores a per-org override that fails the schema and falls through to the global default, which is on. For an entitlement that is right; for this flag it means an org that tried to turn judging off with a stringified boolean kept being judged. The eval policy now refuses on an override it cannot parse, matching what the flag's own comment already claims.
1 parent 9805c0a commit 20c8f9e

3 files changed

Lines changed: 53 additions & 4 deletions

File tree

apps/webapp/app/services/dashboardAgentEvalPolicy.server.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
import { prisma } from "~/db.server";
77
import { logger } from "~/services/logger.server";
8-
import { FEATURE_FLAG } from "~/v3/featureFlags";
8+
import { FEATURE_FLAG, hasUnreadableTurnEvalsOverride } from "~/v3/featureFlags";
99
import { makeFlag } from "~/v3/featureFlags.server";
1010

1111
/** Judging is on unless an org turns it off. */
@@ -31,12 +31,17 @@ export async function orgAllowsDashboardAgentTurnEvals(params: {
3131
});
3232
if (!org) return false;
3333

34+
const overrides = (org.featureFlags as Record<string, unknown>) ?? {};
35+
// `flag()` ignores an override the schema rejects and falls through to the global default,
36+
// which is on — so an org that tried to turn judging off would keep being judged.
37+
if (hasUnreadableTurnEvalsOverride(overrides)) return false;
38+
3439
const flag = makeFlag();
3540
return Boolean(
3641
await flag({
3742
key: FEATURE_FLAG.dashboardAgentTurnEvalsEnabled,
3843
defaultValue: DEFAULT_TURN_EVALS_ENABLED,
39-
overrides: (org.featureFlags as Record<string, unknown>) ?? {},
44+
overrides,
4045
})
4146
);
4247
} catch (error) {

apps/webapp/app/v3/featureFlags.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,26 @@ export function resolveInternalApiOriginEnabled({
173173
return globalDefault;
174174
}
175175

176+
/**
177+
* Whether the org set `dashboardAgentTurnEvalsEnabled` to something the schema rejects.
178+
* That flag is a consent switch, not an entitlement, so an unreadable override must not fall
179+
* through to the global default the way `resolveInternalApiOriginEnabled` does: the org that
180+
* wrote it was trying to say something, and the only safe reading of an unknown answer is no.
181+
*/
182+
export function hasUnreadableTurnEvalsOverride(orgFeatureFlags: unknown): boolean {
183+
if (!orgFeatureFlags || typeof orgFeatureFlags !== "object" || Array.isArray(orgFeatureFlags)) {
184+
return false;
185+
}
186+
187+
const override = (orgFeatureFlags as Record<string, unknown>)[
188+
FEATURE_FLAG.dashboardAgentTurnEvalsEnabled
189+
];
190+
if (override === undefined) return false;
191+
192+
return !FeatureFlagCatalog[FEATURE_FLAG.dashboardAgentTurnEvalsEnabled].safeParse(override)
193+
.success;
194+
}
195+
176196
export type FlagControlType =
177197
| { type: "boolean" }
178198
| { type: "enum"; options: string[] }

apps/webapp/test/featureFlags.test.ts

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@
44
// call counted by a delegating wrapper around the real client.
55
import type { PrismaClient } from "@trigger.dev/database";
66
import { postgresTest } from "@internal/testcontainers";
7-
import { describe, expect, vi } from "vitest";
7+
import { describe, expect, it, vi } from "vitest";
88
import type { PrismaClientOrTransaction } from "~/db.server";
9-
import { FEATURE_FLAG } from "~/v3/featureFlags";
9+
import { FEATURE_FLAG, hasUnreadableTurnEvalsOverride } from "~/v3/featureFlags";
1010
import { makeFlag, makeSetFlag } from "~/v3/featureFlags.server";
1111

1212
vi.setConfig({ testTimeout: 60_000 });
@@ -93,3 +93,27 @@ describe("flag() override resolution", () => {
9393
expect(calls.findFirst).toBe(1);
9494
});
9595
});
96+
97+
// The fall-through above is right for an entitlement and wrong for consent: judging sends the
98+
// turn to a third-party model, so the eval flag refuses on an override it cannot read.
99+
describe("hasUnreadableTurnEvalsOverride", () => {
100+
it("is false when the org has no opinion", () => {
101+
expect(hasUnreadableTurnEvalsOverride({})).toBe(false);
102+
expect(hasUnreadableTurnEvalsOverride(null)).toBe(false);
103+
expect(hasUnreadableTurnEvalsOverride([])).toBe(false);
104+
expect(hasUnreadableTurnEvalsOverride({ somethingElse: "nonsense" })).toBe(false);
105+
});
106+
107+
it("is false for a real boolean, either way", () => {
108+
expect(hasUnreadableTurnEvalsOverride({ [KEY]: false })).toBe(false);
109+
expect(hasUnreadableTurnEvalsOverride({ [KEY]: true })).toBe(false);
110+
});
111+
112+
// The dangerous one: `flag()` would drop "false" and hand back the global default, which is on.
113+
it("is true for a stringified boolean or any other garbage", () => {
114+
expect(hasUnreadableTurnEvalsOverride({ [KEY]: "false" })).toBe(true);
115+
expect(hasUnreadableTurnEvalsOverride({ [KEY]: "true" })).toBe(true);
116+
expect(hasUnreadableTurnEvalsOverride({ [KEY]: 0 })).toBe(true);
117+
expect(hasUnreadableTurnEvalsOverride({ [KEY]: null })).toBe(true);
118+
});
119+
});

0 commit comments

Comments
 (0)