fix(flags): guard inherited rules context attributes

Author: btthomas

packages/core/src/flags/configuration/__tests__/rules.test.ts

@@ -28,7 +28,7 @@ describe('rules configuration', () => {
                     enabled: true
                 }
             })
-        ).toEqual({
+        ).toMatchObject({
             targetingKey: 'user-1',
             country: 'US',
             enabled: true
@@ -46,6 +46,81 @@ describe('rules configuration', () => {
         );
     });
 
+    it.each(['constructor', 'toString'])(
+        'shadows an absent inherited %s context attribute',
+        attribute => {
+            const rulesContext = toRulesEvaluationContext({
+                targetingKey: 'user-1'
+            });
+
+            expect(Object.getPrototypeOf(rulesContext)).toBeNull();
+            expect(
+                Object.prototype.hasOwnProperty.call(rulesContext, attribute)
+            ).toBe(true);
+            expect(rulesContext[attribute]).toBeUndefined();
+
+            const configuration = buildRulesConfiguration();
+            const condition =
+                configuration.flags['dynamic-flag'].allocations[0].rules?.[0]
+                    .conditions[0];
+            if (!condition) {
+                throw new Error('The fixture has no condition.');
+            }
+            condition.attribute = attribute;
+            condition.value = [
+                String(({} as Record<string, unknown>)[attribute])
+            ];
+
+            expect(
+                flaggingCoreRulesEngine.evaluate({
+                    configuration,
+                    type: 'boolean',
+                    flagKey: 'dynamic-flag',
+                    defaultValue: false,
+                    context: rulesContext,
+                    logger: getNoopRulesLogger()
+                })
+            ).toMatchObject({ value: false, reason: 'DEFAULT' });
+        }
+    );
+
+    it.each(['constructor', 'toString'])(
+        'preserves an explicit own %s context attribute',
+        attribute => {
+            const attributes = Object.create(null) as Record<string, string>;
+            attributes[attribute] = 'customer-value';
+
+            const rulesContext = toRulesEvaluationContext({
+                targetingKey: 'user-1',
+                attributes
+            });
+            expect(
+                Object.prototype.hasOwnProperty.call(rulesContext, attribute)
+            ).toBe(true);
+            expect(rulesContext[attribute]).toBe('customer-value');
+            const configuration = buildRulesConfiguration();
+            const condition =
+                configuration.flags['dynamic-flag'].allocations[0].rules?.[0]
+                    .conditions[0];
+            if (!condition) {
+                throw new Error('The fixture has no condition.');
+            }
+            condition.attribute = attribute;
+            condition.value = ['customer-value'];
+
+            expect(
+                flaggingCoreRulesEngine.evaluate({
+                    configuration,
+                    type: 'boolean',
+                    flagKey: 'dynamic-flag',
+                    defaultValue: false,
+                    context: rulesContext,
+                    logger: getNoopRulesLogger()
+                })
+            ).toMatchObject({ value: true, reason: 'TARGETING_MATCH' });
+        }
+    );
+
     it('clones and freezes a valid rules configuration', () => {
         const source = buildRulesConfiguration();
         const prepared = prepareRulesConfiguration(source);
@@ -200,10 +275,15 @@ describe('rules configuration', () => {
 
     // TODO(FFL-2837): Replace this legacy JSON compatibility test with a
     // generated protobuf fixture after a flagging-core release contains
-    // DataDog/openfeature-js-client#344 through `03cde21`. Round-trip the
+    // DataDog/openfeature-js-client#344 through `5a5511e`. Round-trip the
     // generated fixture and confirm that serialization preserves the unknown field.
     // Add a fixture with an unsupported minimum feature level and require a
-    // flag-scoped `PARSE_ERROR`, not `FLAG_NOT_FOUND`.
+    // flag-scoped `PARSE_ERROR`, not `FLAG_NOT_FOUND`. Also cover unsorted
+    // string and SHA-256 membership indexes, invalid SHA digest lengths, and
+    // semantic-version components at and above the unsigned 64-bit limit.
+    // Confirm that an absent inherited `__proto__` attribute does not match and
+    // that an explicit own `__proto__` context attribute remains usable. The
+    // legacy evaluator's compiled object spread cannot preserve that key.
     it('keeps supported known data when an unknown field is present', () => {
         const source = buildRulesConfiguration();
         (source.flags['dynamic-flag'] as typeof source.flags['dynamic-flag'] & {
@@ -230,7 +310,7 @@ describe('rules configuration', () => {
 
     // TODO(FFL-2837): Replace this unsafe JSON number with an out-of-range
     // protobuf `int64` fixture after flagging-core contains PR #344 at or after
-    // `03cde21`. The generated parser must preserve the source value as `bigint`
+    // `5a5511e`. The generated parser must preserve the source value as `bigint`
     // where supported. Run the same evaluation with
     // global `BigInt` unavailable and require `PARSE_ERROR`, not `GENERAL`.
     it('returns PARSE_ERROR instead of serving an unsafe integer', () => {

packages/core/src/flags/configuration/rules.ts

@@ -14,11 +14,12 @@ import type { EvaluationContext, JsonValue, PrimitiveValue } from '../types';
 
 // TODO(FFL-2837): Replace this legacy UFC v1 alias with
 // `NonNullable<FlagsConfiguration['rules']>['response']` after a flagging-core
-// release contains DataDog/openfeature-js-client#344 through `03cde21` and
-// restores 32-byte SHA digest validation. Keep the `FlagsConfiguration` type
+// release contains DataDog/openfeature-js-client#344 through `5a5511e`.
+// Keep the `FlagsConfiguration` type
 // import on the flagging-core package root. PR #344 preserves protobuf integers
 // as `bigint`, does not call global `BigInt` during safe conversion, and reports
-// unsafe conversions as deterministic per-flag `PARSE_ERROR` results.
+// unsafe conversions and malformed SHA digests as deterministic per-flag
+// `PARSE_ERROR` results.
 type RulesConfigurationResponse = UniversalFlagConfigurationV1;
 
 export type RulesValueType = 'boolean' | 'string' | 'number' | 'object';
@@ -119,18 +120,27 @@ export const toRulesEvaluationContext = (
         attributes.set(key, value);
     }
 
-    return {
-        ...Object.fromEntries(attributes),
-        targetingKey: context.targetingKey
-    };
+    // TODO(FFL-2837): Delete the inherited-name shadows after a flagging-core
+    // release contains DataDog/openfeature-js-client#344 through `aa93230` and
+    // this adapter uses its generated protobuf evaluator. That evaluator uses
+    // own-property lookup for condition and shard context attributes.
+    const rulesContext = Object.create(null) as RulesEvaluationContext;
+    for (const key of Object.getOwnPropertyNames(Object.prototype)) {
+        rulesContext[key] = undefined;
+    }
+    for (const [key, value] of attributes) {
+        rulesContext[key] = value;
+    }
+    rulesContext.targetingKey = context.targetingKey;
+
+    return rulesContext;
 };
 
 const hasOwn = (value: object, key: PropertyKey): boolean =>
     Object.prototype.hasOwnProperty.call(value, key);
 
 // TODO(FFL-2837): Delete this compatibility error store after a flagging-core
-// release contains DataDog/openfeature-js-client#344 through `03cde21` and the
-// required SHA digest validation follow-up.
+// release contains DataDog/openfeature-js-client#344 through `5a5511e`.
 // The generated protobuf evaluator validates the requested flag and the data
 // that evaluation reaches. It does not build this error map during parsing.
 // It returns deterministic `PARSE_ERROR` results, including for an integer that
@@ -209,7 +219,7 @@ const validateCondition = (value: unknown): string | undefined => {
             try {
                 // TODO(FFL-2837): Define a bounded regular expression policy before
                 // dynamic offline rules leave draft state. Upstream PR #344 through
-                // `03cde21` compiles protobuf regular expressions lazily and caches
+                // `5a5511e` compiles protobuf regular expressions lazily and caches
                 // them by configuration and index, but it does not limit patterns.
                 RegExp(value.value); // dd-iac-scan ignore-line
             } catch {
@@ -464,8 +474,7 @@ export const prepareRulesConfiguration = (
     const clone = cloneValue(value);
 
     // TODO(FFL-2837): Delete this legacy JSON clone and validator after a
-    // flagging-core release contains upstream PR #344 through `03cde21` and the
-    // required SHA digest validation follow-up. That
+    // flagging-core release contains upstream PR #344 through `5a5511e`. That
     // implementation preserves protobuf integers as `bigint` and validates only
     // the requested flag data that evaluation reaches. It does not call global
     // `BigInt` when it returns a deterministic per-flag error for an unsafe number.
@@ -520,7 +529,7 @@ const normalizeVariationType = (
 
 // TODO(FFL-2837): Delete this legacy UFC v1 metadata fallback after the
 // flagging-core dependency contains DataDog/openfeature-js-client#344 through
-// `03cde21`. The protobuf evaluator maps only safely represented integer
+// `5a5511e`. The protobuf evaluator maps only safely represented integer
 // variations, and all numeric variations, to the OpenFeature type `number`.
 const recoverVariationType = (
     configuration: RulesConfigurationResponse,
@@ -547,7 +556,7 @@ export const flaggingCoreRulesEngine: RulesEngine = {
 
         // TODO(FFL-2837): Delete this local compatibility guard after the
         // flagging-core dependency contains DataDog/openfeature-js-client#344
-        // through `03cde21`. Keep the reserved-name contract tests.
+        // through `5a5511e`. Keep the reserved-name contract tests.
         if (!hasOwn(flags, request.flagKey)) {
             return {
                 value: request.defaultValue,
@@ -558,7 +567,7 @@ export const flaggingCoreRulesEngine: RulesEngine = {
         }
 
         // TODO(FFL-2837): Delete this compatibility check with the local error
-        // store after the published PR #344 evaluator through `03cde21` validates
+        // store after the published PR #344 evaluator through `5a5511e` validates
         // reached flag data and reports deterministic flag-scoped errors, including
         // unsupported feature levels and unsafe integer conversions with and
         // without global `BigInt` when supported.

packages/core/src/flags/configuration/wire.ts

@@ -24,17 +24,16 @@ import type {
 
 // TODO(FFL-2837): Delete the pending `rulesBased` types, reader, and wrappers
 // after a flagging-core release contains DataDog/openfeature-js-client#344
-// through `03cde21` plus the required SHA digest follow-up. The `03cde21` tree
-// is identical to the previous `939da97` tree. Its final `1db13d4` and
-// `03cde21` commits update generated Node-server artifacts and browser test
-// isolation; they do not change this React Native boundary.
+// through `5a5511e`.
 // Import and re-export the wire functions and `FlagsConfigurationWire` type from
-// `@datadog/flagging-core/configuration`. Keep `FlagsConfiguration` and the rules
-// evaluator on the package root. The new `@datadog/flagging-core/precomputed`
+// `@datadog/flagging-core/configuration`. Do not use the deprecated package-root
+// aliases because they parse precomputed data only and ignore rules. Keep
+// `FlagsConfiguration` and the rules evaluator on the package root. The
+// `@datadog/flagging-core/precomputed`
 // subpath is protobuf-free, ignores rules, and is not the parser for this module.
-// PR #336 through `772167b` adds browser providers and shared lifecycle error
-// selection. Its tree is identical to the previous `6d3d6a4` tree, so it does
-// not change this core parser boundary. Do not import
+// PR #336 through `dde93ea` adds browser providers and shared lifecycle error
+// selection. Its latest commit removes unrelated `extraLogging` test coverage
+// and does not change this core parser boundary. Do not import
 // `@datadog/openfeature-browser` in React Native.
 // Use `FlagsConfiguration.rules`. The distribution layer must put one base64
 // encoding of the raw dd-source#34959 protobuf response in the version 1
@@ -43,7 +42,8 @@ import type {
 // service transport or envelope construction here. PR #344 preserves decoded
 // protobuf flags and integers. Its evaluator reports invalid reached data,
 // unsupported feature levels, and unsafe integer conversion as deterministic
-// flag-scoped `PARSE_ERROR` results.
+// flag-scoped `PARSE_ERROR` results. It also validates membership ordering,
+// semantic-version bounds, and 32-byte SHA-256 digests.
 type PendingRulesConfiguration = FlagsConfiguration & {
     configurationError?: string;
     rulesError?: string;
@@ -122,9 +122,9 @@ export const configurationFromString = (source: string): FlagsConfiguration => {
     // validator that PR #344 removed in favor of the Protobuf-ES decoder. The
     // published parser must also include PR #344's unknown-field tolerance,
     // unknown-field serialization, and lossless integer parsing through
-    // `03cde21`. Its safe-integer conversion does not require global `BigInt`.
+    // `5a5511e`. Its safe-integer conversion does not require global `BigInt`.
     // TODO(FFL-2837): Delete this parse-error compatibility behavior when the
-    // dependency contains PR #344 through `03cde21`. The upstream parser uses
+    // dependency contains PR #344 through `5a5511e`. The upstream parser uses
     // `configurationError` for an invalid envelope and `rulesError` for an
     // invalid rules entry or response. It keeps a valid sibling branch.
     const pendingRules = readPendingRulesWire(source);
@@ -155,8 +155,8 @@ export const configurationToString = (
     const pendingConfiguration = configuration as PendingRulesConfiguration;
 
     // TODO(FFL-2837): Delete this legacy serialization wrapper with the pending
-    // types above after the dependency contains PR #344 through `03cde21` and
-    // its required SHA digest follow-up. The upstream serializer encodes generated protobuf
+    // types above after the dependency contains PR #344 through `5a5511e`.
+    // The upstream serializer encodes generated protobuf
     // rules back to base64 and preserves unknown protobuf fields.
     // This temporary UFC v1 shim serializes its legacy JSON response instead.
     if (pendingConfiguration.rulesBased) {