From 4e4a35d99744b2752d7ee1909d72f1230bea60cb Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Wed, 1 Jul 2026 08:51:26 -0400 Subject: [PATCH 1/4] fix(openfeature): bound context flattening depth and detect cycles flattenRecursive walked map[string]any / []any evaluation context with no depth limit and no cycle detection. An attacker-influenced context with deep nesting or a self-referential map could recurse until the Go runtime hit a fatal, unrecoverable stack overflow (a process-crash DoS, worse than a degraded telemetry path since recover() cannot catch it); a wide context was also fully materialized before the field cap applied. Add maxContextDepth (32) plus stack-based cycle detection for the only self-referenceable container types (map[string]any, []any), threaded through the flatten helpers. This hardens both the EVP flagevaluation and the older exposure flattening paths. Output is unchanged for legitimate contexts (depth <= 32, no cycles); shared sub-trees (diamonds) are still fully flattened. dd-trace-rb fixed the same defect class in #5896 (commit f8e718). Codex-scan finding: d4c4823ac5148191bd9869bb77f0227e Refs: APMSP-3616, FFL-2651 --- openfeature/flatten.go | 120 ++++++++++++++++++++++++++---------- openfeature/flatten_test.go | 98 +++++++++++++++++++++++++++++ 2 files changed, 186 insertions(+), 32 deletions(-) create mode 100644 openfeature/flatten_test.go diff --git a/openfeature/flatten.go b/openfeature/flatten.go index 84d89043c25..5ac7df3802e 100644 --- a/openfeature/flatten.go +++ b/openfeature/flatten.go @@ -7,12 +7,23 @@ package openfeature import ( "fmt" + "reflect" "strconv" "strings" "github.com/DataDog/dd-trace-go/v2/internal/log" ) +// maxContextDepth bounds the recursion depth of context flattening. +// +// The evaluation context is attacker-influenceable: a deeply nested or self-referential +// map[string]any / []any would otherwise recurse without limit. In Go a stack overflow is a +// fatal runtime error ("goroutine stack exceeds ... limit") that cannot be caught by recover(), +// so an unbounded traversal is a process-crash DoS rather than a mere degraded telemetry path. +// 32 is far deeper than any legitimate evaluation context and matches the other SDKs +// (see dd-trace-rb Aggregator::MAX_CONTEXT_DEPTH). +const maxContextDepth = 32 + // flattenRecursive recursively flattens nested attributes into a single-level map // using dot notation for nested keys. This ensures that subject attributes in exposure // events comply with the EVP intake schema which does not allow nested objects. @@ -26,62 +37,90 @@ import ( // {"user.id": "123", "user.email": "test@example.com"} // // The flattening is applied during both flag evaluation and exposure event creation. +// +// Traversal is bounded: recursion stops at maxContextDepth and cycles through +// map[string]any / []any containers are detected and skipped so an attacker-supplied +// deeply nested or self-referential context cannot exhaust the stack. func flattenRecursive(prefix string, value any, result map[string]any) { + flattenValue(prefix, value, result, nil, 0) +} + +// flattenValue is the depth- and cycle-bounded core of flattenRecursive. +// +// seen tracks the identity of map[string]any / []any containers currently on the recursion +// stack (they are the only element types that can form a cycle, since their values are `any`). +// It is allocated lazily on the first such container and threaded through the recursion; a +// container is added before descending and removed on the way out, so shared sub-trees (a +// diamond) are still fully flattened while genuine cycles are broken. +func flattenValue(prefix string, value any, result map[string]any, seen map[uintptr]struct{}, depth int) { + if depth > maxContextDepth { + log.Debug("openfeature: skipping attribute %q: context nesting exceeds max depth %d", prefix, maxContextDepth) + return + } + switch v := value.(type) { case map[string]any: - flattenRecursiveMap(prefix, v, result) + if seen = enterContainer(seen, v, prefix); seen == nil { + return + } + flattenRecursiveMap(prefix, v, result, seen, depth) + delete(seen, reflect.ValueOf(v).Pointer()) case map[string]string: - flattenRecursiveMap(prefix, v, result) + flattenRecursiveMap(prefix, v, result, seen, depth) case map[string]uint: - flattenRecursiveMap(prefix, v, result) + flattenRecursiveMap(prefix, v, result, seen, depth) case map[string]int: - flattenRecursiveMap(prefix, v, result) + flattenRecursiveMap(prefix, v, result, seen, depth) case map[string]int64: - flattenRecursiveMap(prefix, v, result) + flattenRecursiveMap(prefix, v, result, seen, depth) case map[string]int32: - flattenRecursiveMap(prefix, v, result) + flattenRecursiveMap(prefix, v, result, seen, depth) case map[string]uint64: - flattenRecursiveMap(prefix, v, result) + flattenRecursiveMap(prefix, v, result, seen, depth) case map[string]uint32: - flattenRecursiveMap(prefix, v, result) + flattenRecursiveMap(prefix, v, result, seen, depth) case map[string]int16: - flattenRecursiveMap(prefix, v, result) + flattenRecursiveMap(prefix, v, result, seen, depth) case map[string]int8: - flattenRecursiveMap(prefix, v, result) + flattenRecursiveMap(prefix, v, result, seen, depth) case map[string]uint16: - flattenRecursiveMap(prefix, v, result) + flattenRecursiveMap(prefix, v, result, seen, depth) case map[string]uint8: - flattenRecursiveMap(prefix, v, result) + flattenRecursiveMap(prefix, v, result, seen, depth) case map[string]float64: - flattenRecursiveMap(prefix, v, result) + flattenRecursiveMap(prefix, v, result, seen, depth) case map[string]bool: - flattenRecursiveMap(prefix, v, result) + flattenRecursiveMap(prefix, v, result, seen, depth) case map[string]float32: - flattenRecursiveMap(prefix, v, result) + flattenRecursiveMap(prefix, v, result, seen, depth) + case []any: + if seen = enterContainer(seen, v, prefix); seen == nil { + return + } + flattenRecursiveArray(prefix, v, result, seen, depth) + delete(seen, reflect.ValueOf(v).Pointer()) case []string: - flattenRecursiveArray(prefix, v, result) + flattenRecursiveArray(prefix, v, result, seen, depth) case []int: - flattenRecursiveArray(prefix, v, result) + flattenRecursiveArray(prefix, v, result, seen, depth) case []int64: - flattenRecursiveArray(prefix, v, result) + flattenRecursiveArray(prefix, v, result, seen, depth) case []int32: - flattenRecursiveArray(prefix, v, result) + flattenRecursiveArray(prefix, v, result, seen, depth) case []uint64: - flattenRecursiveArray(prefix, v, result) + flattenRecursiveArray(prefix, v, result, seen, depth) case []uint32: - flattenRecursiveArray(prefix, v, result) + flattenRecursiveArray(prefix, v, result, seen, depth) case []int16: - flattenRecursiveArray(prefix, v, result) + flattenRecursiveArray(prefix, v, result, seen, depth) case []uint16: - flattenRecursiveArray(prefix, v, result) + flattenRecursiveArray(prefix, v, result, seen, depth) case []float64: - flattenRecursiveArray(prefix, v, result) + flattenRecursiveArray(prefix, v, result, seen, depth) case []bool: - flattenRecursiveArray(prefix, v, result) + flattenRecursiveArray(prefix, v, result, seen, depth) case []float32: - flattenRecursiveArray(prefix, v, result) - case []any: - flattenRecursiveArray(prefix, v, result) + flattenRecursiveArray(prefix, v, result, seen, depth) case []byte: result[prefix] = string(v) case fmt.Stringer: @@ -93,20 +132,37 @@ func flattenRecursive(prefix string, value any, result map[string]any) { } } -func flattenRecursiveMap[T any](prefix string, v map[string]T, result map[string]any) { +// enterContainer records the identity of a map[string]any / []any container before descending +// into it so a self-referential context is not traversed forever. It returns the (possibly +// newly-allocated) seen set to use for the descent, or nil if the container is already on the +// current recursion stack (a cycle), in which case the caller must skip it. +func enterContainer(seen map[uintptr]struct{}, container any, prefix string) map[uintptr]struct{} { + ptr := reflect.ValueOf(container).Pointer() + if _, ok := seen[ptr]; ok { + log.Debug("openfeature: skipping attribute %q: cyclic evaluation context reference", prefix) + return nil + } + if seen == nil { + seen = make(map[uintptr]struct{}, 1) + } + seen[ptr] = struct{}{} + return seen +} + +func flattenRecursiveMap[T any](prefix string, v map[string]T, result map[string]any, seen map[uintptr]struct{}, depth int) { for key, val := range v { newPrefix := key if prefix != "" { newPrefix = prefix + "." + key } - flattenRecursive(newPrefix, val, result) + flattenValue(newPrefix, val, result, seen, depth+1) } } -func flattenRecursiveArray[T any](prefix string, v []T, result map[string]any) { +func flattenRecursiveArray[T any](prefix string, v []T, result map[string]any, seen map[uintptr]struct{}, depth int) { for i, item := range v { flattenKey := prefix + "." + strconv.Itoa(i) - flattenRecursive(flattenKey, item, result) + flattenValue(flattenKey, item, result, seen, depth+1) } } diff --git a/openfeature/flatten_test.go b/openfeature/flatten_test.go new file mode 100644 index 00000000000..77ed1dd65ef --- /dev/null +++ b/openfeature/flatten_test.go @@ -0,0 +1,98 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2025 Datadog, Inc. + +package openfeature + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" +) + +// TestFlattenContextNested is a regression guard: legitimate nested context still flattens to +// dot notation exactly as before the depth/cycle hardening was added. +func TestFlattenContextNested(t *testing.T) { + in := map[string]any{ + "user": map[string]any{"id": "123", "email": "a@b.com"}, + "plan": "pro", + } + got := flattenContext(in) + assert.Equal(t, map[string]any{ + "user.id": "123", + "user.email": "a@b.com", + "plan": "pro", + }, got) +} + +// TestFlattenContextSelfReferenceDoesNotCrash verifies that a directly self-referential +// context does not recurse forever. In Go a stack overflow is fatal (not recoverable), so a +// missing guard would crash the whole process rather than fail this assertion. +func TestFlattenContextSelfReferenceDoesNotCrash(t *testing.T) { + m := map[string]any{"name": "leo"} + m["self"] = m // cycle: m -> m + + got := flattenContext(m) + + assert.Equal(t, "leo", got["name"]) + // The cyclic back-reference must be broken, never expanded. + for k := range got { + assert.NotContains(t, k, "self.self") + } +} + +// TestFlattenContextIndirectCycleDoesNotCrash covers an a -> b -> a cycle across two maps. +func TestFlattenContextIndirectCycleDoesNotCrash(t *testing.T) { + a := map[string]any{"leaf": "x"} + b := map[string]any{"a": a} + a["b"] = b // a -> b -> a + + got := flattenContext(map[string]any{"root": a}) + + assert.Equal(t, "x", got["root.leaf"]) +} + +// TestFlattenContextSharedSubtreeNotTreatedAsCycle ensures cycle detection does not eat a +// legitimate diamond (the same sub-map referenced from two sibling keys). +func TestFlattenContextSharedSubtreeNotTreatedAsCycle(t *testing.T) { + shared := map[string]any{"v": "1"} + in := map[string]any{"x": shared, "y": shared} + + got := flattenContext(in) + + assert.Equal(t, "1", got["x.v"]) + assert.Equal(t, "1", got["y.v"]) +} + +// TestFlattenContextDeepMapBounded verifies nesting beyond maxContextDepth is dropped rather +// than exhausting the stack. +func TestFlattenContextDeepMapBounded(t *testing.T) { + deep := map[string]any{"leaf": "deep-value"} + for i := 0; i < maxContextDepth+10; i++ { + deep = map[string]any{"n": deep} + } + + got := flattenContext(deep) + + // The leaf sits below the depth cap, so it must not appear, and no key may exceed the cap. + for k, v := range got { + assert.NotEqual(t, "deep-value", v) + assert.LessOrEqual(t, strings.Count(k, "."), maxContextDepth) + } +} + +// TestFlattenContextDeepArrayBounded is the array analogue of the deep-map bound. +func TestFlattenContextDeepArrayBounded(t *testing.T) { + var deep any = "leaf-value" + for i := 0; i < maxContextDepth+10; i++ { + deep = []any{deep} + } + + got := flattenContext(map[string]any{"arr": deep}) + + for _, v := range got { + assert.NotEqual(t, "leaf-value", v) + } +} From 74c3a7f2962e4810ecd3679dda5f300d114d095a Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Wed, 1 Jul 2026 09:21:28 -0400 Subject: [PATCH 2/4] refactor(openfeature): simplify context flatten guard to a depth cap Drop the cycle-detection machinery (reflect-based identity tracking, seen set, enterContainer helper) from the previous commit. The fatal failure is the stack overflow, and the depth cap alone prevents it: a self-referential context simply recurses to maxContextDepth and stops. Any bounded junk keys a cycle produces are pruned by the existing 256-field limit. This keeps the switch and helpers structurally identical to the original, adding only a threaded depth int and one guard. --- openfeature/flatten.go | 116 +++++++++++++----------------------- openfeature/flatten_test.go | 27 +++------ 2 files changed, 49 insertions(+), 94 deletions(-) diff --git a/openfeature/flatten.go b/openfeature/flatten.go index 5ac7df3802e..a59e85b550d 100644 --- a/openfeature/flatten.go +++ b/openfeature/flatten.go @@ -7,21 +7,19 @@ package openfeature import ( "fmt" - "reflect" "strconv" "strings" "github.com/DataDog/dd-trace-go/v2/internal/log" ) -// maxContextDepth bounds the recursion depth of context flattening. +// maxContextDepth bounds how deep context flattening will recurse. // // The evaluation context is attacker-influenceable: a deeply nested or self-referential // map[string]any / []any would otherwise recurse without limit. In Go a stack overflow is a -// fatal runtime error ("goroutine stack exceeds ... limit") that cannot be caught by recover(), -// so an unbounded traversal is a process-crash DoS rather than a mere degraded telemetry path. -// 32 is far deeper than any legitimate evaluation context and matches the other SDKs -// (see dd-trace-rb Aggregator::MAX_CONTEXT_DEPTH). +// fatal runtime error that recover() cannot catch, so an unbounded traversal is a process-crash +// DoS. Capping the depth stops the crash (a cycle simply terminates at the cap); 32 is far +// deeper than any real evaluation context and matches the other SDKs. const maxContextDepth = 32 // flattenRecursive recursively flattens nested attributes into a single-level map @@ -37,22 +35,13 @@ const maxContextDepth = 32 // {"user.id": "123", "user.email": "test@example.com"} // // The flattening is applied during both flag evaluation and exposure event creation. -// -// Traversal is bounded: recursion stops at maxContextDepth and cycles through -// map[string]any / []any containers are detected and skipped so an attacker-supplied -// deeply nested or self-referential context cannot exhaust the stack. +// Recursion is bounded by maxContextDepth so an attacker-supplied deeply nested or +// self-referential context cannot exhaust the stack. func flattenRecursive(prefix string, value any, result map[string]any) { - flattenValue(prefix, value, result, nil, 0) + flattenRecursiveDepth(prefix, value, result, 0) } -// flattenValue is the depth- and cycle-bounded core of flattenRecursive. -// -// seen tracks the identity of map[string]any / []any containers currently on the recursion -// stack (they are the only element types that can form a cycle, since their values are `any`). -// It is allocated lazily on the first such container and threaded through the recursion; a -// container is added before descending and removed on the way out, so shared sub-trees (a -// diamond) are still fully flattened while genuine cycles are broken. -func flattenValue(prefix string, value any, result map[string]any, seen map[uintptr]struct{}, depth int) { +func flattenRecursiveDepth(prefix string, value any, result map[string]any, depth int) { if depth > maxContextDepth { log.Debug("openfeature: skipping attribute %q: context nesting exceeds max depth %d", prefix, maxContextDepth) return @@ -60,67 +49,59 @@ func flattenValue(prefix string, value any, result map[string]any, seen map[uint switch v := value.(type) { case map[string]any: - if seen = enterContainer(seen, v, prefix); seen == nil { - return - } - flattenRecursiveMap(prefix, v, result, seen, depth) - delete(seen, reflect.ValueOf(v).Pointer()) + flattenRecursiveMap(prefix, v, result, depth) case map[string]string: - flattenRecursiveMap(prefix, v, result, seen, depth) + flattenRecursiveMap(prefix, v, result, depth) case map[string]uint: - flattenRecursiveMap(prefix, v, result, seen, depth) + flattenRecursiveMap(prefix, v, result, depth) case map[string]int: - flattenRecursiveMap(prefix, v, result, seen, depth) + flattenRecursiveMap(prefix, v, result, depth) case map[string]int64: - flattenRecursiveMap(prefix, v, result, seen, depth) + flattenRecursiveMap(prefix, v, result, depth) case map[string]int32: - flattenRecursiveMap(prefix, v, result, seen, depth) + flattenRecursiveMap(prefix, v, result, depth) case map[string]uint64: - flattenRecursiveMap(prefix, v, result, seen, depth) + flattenRecursiveMap(prefix, v, result, depth) case map[string]uint32: - flattenRecursiveMap(prefix, v, result, seen, depth) + flattenRecursiveMap(prefix, v, result, depth) case map[string]int16: - flattenRecursiveMap(prefix, v, result, seen, depth) + flattenRecursiveMap(prefix, v, result, depth) case map[string]int8: - flattenRecursiveMap(prefix, v, result, seen, depth) + flattenRecursiveMap(prefix, v, result, depth) case map[string]uint16: - flattenRecursiveMap(prefix, v, result, seen, depth) + flattenRecursiveMap(prefix, v, result, depth) case map[string]uint8: - flattenRecursiveMap(prefix, v, result, seen, depth) + flattenRecursiveMap(prefix, v, result, depth) case map[string]float64: - flattenRecursiveMap(prefix, v, result, seen, depth) + flattenRecursiveMap(prefix, v, result, depth) case map[string]bool: - flattenRecursiveMap(prefix, v, result, seen, depth) + flattenRecursiveMap(prefix, v, result, depth) case map[string]float32: - flattenRecursiveMap(prefix, v, result, seen, depth) - case []any: - if seen = enterContainer(seen, v, prefix); seen == nil { - return - } - flattenRecursiveArray(prefix, v, result, seen, depth) - delete(seen, reflect.ValueOf(v).Pointer()) + flattenRecursiveMap(prefix, v, result, depth) case []string: - flattenRecursiveArray(prefix, v, result, seen, depth) + flattenRecursiveArray(prefix, v, result, depth) case []int: - flattenRecursiveArray(prefix, v, result, seen, depth) + flattenRecursiveArray(prefix, v, result, depth) case []int64: - flattenRecursiveArray(prefix, v, result, seen, depth) + flattenRecursiveArray(prefix, v, result, depth) case []int32: - flattenRecursiveArray(prefix, v, result, seen, depth) + flattenRecursiveArray(prefix, v, result, depth) case []uint64: - flattenRecursiveArray(prefix, v, result, seen, depth) + flattenRecursiveArray(prefix, v, result, depth) case []uint32: - flattenRecursiveArray(prefix, v, result, seen, depth) + flattenRecursiveArray(prefix, v, result, depth) case []int16: - flattenRecursiveArray(prefix, v, result, seen, depth) + flattenRecursiveArray(prefix, v, result, depth) case []uint16: - flattenRecursiveArray(prefix, v, result, seen, depth) + flattenRecursiveArray(prefix, v, result, depth) case []float64: - flattenRecursiveArray(prefix, v, result, seen, depth) + flattenRecursiveArray(prefix, v, result, depth) case []bool: - flattenRecursiveArray(prefix, v, result, seen, depth) + flattenRecursiveArray(prefix, v, result, depth) case []float32: - flattenRecursiveArray(prefix, v, result, seen, depth) + flattenRecursiveArray(prefix, v, result, depth) + case []any: + flattenRecursiveArray(prefix, v, result, depth) case []byte: result[prefix] = string(v) case fmt.Stringer: @@ -132,37 +113,20 @@ func flattenValue(prefix string, value any, result map[string]any, seen map[uint } } -// enterContainer records the identity of a map[string]any / []any container before descending -// into it so a self-referential context is not traversed forever. It returns the (possibly -// newly-allocated) seen set to use for the descent, or nil if the container is already on the -// current recursion stack (a cycle), in which case the caller must skip it. -func enterContainer(seen map[uintptr]struct{}, container any, prefix string) map[uintptr]struct{} { - ptr := reflect.ValueOf(container).Pointer() - if _, ok := seen[ptr]; ok { - log.Debug("openfeature: skipping attribute %q: cyclic evaluation context reference", prefix) - return nil - } - if seen == nil { - seen = make(map[uintptr]struct{}, 1) - } - seen[ptr] = struct{}{} - return seen -} - -func flattenRecursiveMap[T any](prefix string, v map[string]T, result map[string]any, seen map[uintptr]struct{}, depth int) { +func flattenRecursiveMap[T any](prefix string, v map[string]T, result map[string]any, depth int) { for key, val := range v { newPrefix := key if prefix != "" { newPrefix = prefix + "." + key } - flattenValue(newPrefix, val, result, seen, depth+1) + flattenRecursiveDepth(newPrefix, val, result, depth+1) } } -func flattenRecursiveArray[T any](prefix string, v []T, result map[string]any, seen map[uintptr]struct{}, depth int) { +func flattenRecursiveArray[T any](prefix string, v []T, result map[string]any, depth int) { for i, item := range v { flattenKey := prefix + "." + strconv.Itoa(i) - flattenValue(flattenKey, item, result, seen, depth+1) + flattenRecursiveDepth(flattenKey, item, result, depth+1) } } diff --git a/openfeature/flatten_test.go b/openfeature/flatten_test.go index 77ed1dd65ef..be31d0db970 100644 --- a/openfeature/flatten_test.go +++ b/openfeature/flatten_test.go @@ -13,7 +13,7 @@ import ( ) // TestFlattenContextNested is a regression guard: legitimate nested context still flattens to -// dot notation exactly as before the depth/cycle hardening was added. +// dot notation exactly as before the depth cap was added. func TestFlattenContextNested(t *testing.T) { in := map[string]any{ "user": map[string]any{"id": "123", "email": "a@b.com"}, @@ -27,9 +27,10 @@ func TestFlattenContextNested(t *testing.T) { }, got) } -// TestFlattenContextSelfReferenceDoesNotCrash verifies that a directly self-referential -// context does not recurse forever. In Go a stack overflow is fatal (not recoverable), so a -// missing guard would crash the whole process rather than fail this assertion. +// TestFlattenContextSelfReferenceDoesNotCrash verifies that a directly self-referential context +// terminates instead of recursing forever. In Go a stack overflow is fatal (recover() cannot +// catch it), so without the depth cap this would crash the whole process rather than fail the +// assertion. The cyclic branch is bounded to maxContextDepth levels. func TestFlattenContextSelfReferenceDoesNotCrash(t *testing.T) { m := map[string]any{"name": "leo"} m["self"] = m // cycle: m -> m @@ -37,9 +38,8 @@ func TestFlattenContextSelfReferenceDoesNotCrash(t *testing.T) { got := flattenContext(m) assert.Equal(t, "leo", got["name"]) - // The cyclic back-reference must be broken, never expanded. for k := range got { - assert.NotContains(t, k, "self.self") + assert.LessOrEqual(t, strings.Count(k, "."), maxContextDepth) } } @@ -52,18 +52,9 @@ func TestFlattenContextIndirectCycleDoesNotCrash(t *testing.T) { got := flattenContext(map[string]any{"root": a}) assert.Equal(t, "x", got["root.leaf"]) -} - -// TestFlattenContextSharedSubtreeNotTreatedAsCycle ensures cycle detection does not eat a -// legitimate diamond (the same sub-map referenced from two sibling keys). -func TestFlattenContextSharedSubtreeNotTreatedAsCycle(t *testing.T) { - shared := map[string]any{"v": "1"} - in := map[string]any{"x": shared, "y": shared} - - got := flattenContext(in) - - assert.Equal(t, "1", got["x.v"]) - assert.Equal(t, "1", got["y.v"]) + for k := range got { + assert.LessOrEqual(t, strings.Count(k, "."), maxContextDepth) + } } // TestFlattenContextDeepMapBounded verifies nesting beyond maxContextDepth is dropped rather From b799b15c704f175538779c24f74b3e56a65b00a2 Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Wed, 1 Jul 2026 12:31:17 -0400 Subject: [PATCH 3/4] test(openfeature): use range-over-int loops in flatten tests Satisfies golangci-lint modernize (rangeint); the loop counters were unused. --- openfeature/flatten_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openfeature/flatten_test.go b/openfeature/flatten_test.go index be31d0db970..d67db210aad 100644 --- a/openfeature/flatten_test.go +++ b/openfeature/flatten_test.go @@ -61,7 +61,7 @@ func TestFlattenContextIndirectCycleDoesNotCrash(t *testing.T) { // than exhausting the stack. func TestFlattenContextDeepMapBounded(t *testing.T) { deep := map[string]any{"leaf": "deep-value"} - for i := 0; i < maxContextDepth+10; i++ { + for range maxContextDepth + 10 { deep = map[string]any{"n": deep} } @@ -77,7 +77,7 @@ func TestFlattenContextDeepMapBounded(t *testing.T) { // TestFlattenContextDeepArrayBounded is the array analogue of the deep-map bound. func TestFlattenContextDeepArrayBounded(t *testing.T) { var deep any = "leaf-value" - for i := 0; i < maxContextDepth+10; i++ { + for range maxContextDepth + 10 { deep = []any{deep} } From 244502d64fa6a93c1ba10d4a1044546432df7e03 Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Wed, 1 Jul 2026 12:41:51 -0400 Subject: [PATCH 4/4] fix(openfeature): restore cycle detection and cap total flatten fields The depth cap alone fixes the fatal stack overflow but not the memory/CPU amplification the security report also called out: a context that shares a child map/slice across many keys (a DAG) fans out ~2^depth before the 256-field prune runs, and a self-referential map produces bounded-but-junk keys. Restore the two guards from the original finding alongside the depth cap: - cycle detection: track map[string]any / []any identity on the recursion stack and skip references already in progress (breaks true cycles; diamonds still flatten fully since identity is removed on the way out). - maxFlattenFields (65536): a total-field safety ceiling that bounds DAG fan-out. It sits far above the 256-field intake prune, so real contexts are unaffected and the deterministic prune that builds the aggregation bucket key is unchanged; only pathological amplification is truncated. All of this runs on the background aggregation worker, not the evaluation hot path, so there is no per-evaluation cost. Tests are table-driven in flatten_test.go: normal nesting, direct/indirect cycles, diamond (not a cycle), depth-bounded map/array, and field-ceiling fan-out. Refs: APMSP-3616, FFL-2651 --- openfeature/flatten.go | 131 ++++++++++++++++-------- openfeature/flatten_test.go | 199 +++++++++++++++++++++++------------- 2 files changed, 215 insertions(+), 115 deletions(-) diff --git a/openfeature/flatten.go b/openfeature/flatten.go index a59e85b550d..7594161852a 100644 --- a/openfeature/flatten.go +++ b/openfeature/flatten.go @@ -7,20 +7,36 @@ package openfeature import ( "fmt" + "reflect" "strconv" "strings" "github.com/DataDog/dd-trace-go/v2/internal/log" ) -// maxContextDepth bounds how deep context flattening will recurse. -// -// The evaluation context is attacker-influenceable: a deeply nested or self-referential -// map[string]any / []any would otherwise recurse without limit. In Go a stack overflow is a -// fatal runtime error that recover() cannot catch, so an unbounded traversal is a process-crash -// DoS. Capping the depth stops the crash (a cycle simply terminates at the cap); 32 is far -// deeper than any real evaluation context and matches the other SDKs. -const maxContextDepth = 32 +const ( + // maxContextDepth bounds how deep context flattening will recurse. + // + // The evaluation context is attacker-influenceable: a deeply nested map[string]any / []any + // would otherwise recurse without limit. In Go a stack overflow is a fatal runtime error that + // recover() cannot catch, so an unbounded traversal is a process-crash DoS. 32 is far deeper + // than any real evaluation context and matches the other SDKs. + maxContextDepth = 32 + + // maxFlattenFields is a safety ceiling on the TOTAL number of fields a single flatten produces. + // + // Depth capping and cycle detection stop unbounded depth and true cycles, but a context that + // shares the same child map/slice across many keys (a DAG) still fans out multiplicatively — + // e.g. branching-2 nesting expands ~2^depth leaves before any downstream cap applies. This + // bounds that memory/CPU amplification. + // + // It sits far above maxContextFields (the 256-field intake prune), so it never affects a real + // context: legitimate inputs flatten well below it and the deterministic 256-field prune that + // builds the aggregation bucket key is unchanged; only pathological amplification is truncated. + // Flattening runs on the background aggregation worker, not the evaluation hot path, so this + // adds no per-evaluation cost. + maxFlattenFields = 1 << 16 // 65536 +) // flattenRecursive recursively flattens nested attributes into a single-level map // using dot notation for nested keys. This ensures that subject attributes in exposure @@ -35,13 +51,17 @@ const maxContextDepth = 32 // {"user.id": "123", "user.email": "test@example.com"} // // The flattening is applied during both flag evaluation and exposure event creation. -// Recursion is bounded by maxContextDepth so an attacker-supplied deeply nested or -// self-referential context cannot exhaust the stack. +// Traversal is bounded so an attacker-supplied context cannot exhaust the stack or memory: +// recursion stops at maxContextDepth, cycles through map[string]any / []any containers are +// detected and skipped, and the total field count is capped at maxFlattenFields. func flattenRecursive(prefix string, value any, result map[string]any) { - flattenRecursiveDepth(prefix, value, result, 0) + flattenRecursiveDepth(prefix, value, result, nil, 0) } -func flattenRecursiveDepth(prefix string, value any, result map[string]any, depth int) { +func flattenRecursiveDepth(prefix string, value any, result map[string]any, seen map[uintptr]struct{}, depth int) { + if len(result) >= maxFlattenFields { + return + } if depth > maxContextDepth { log.Debug("openfeature: skipping attribute %q: context nesting exceeds max depth %d", prefix, maxContextDepth) return @@ -49,59 +69,82 @@ func flattenRecursiveDepth(prefix string, value any, result map[string]any, dept switch v := value.(type) { case map[string]any: - flattenRecursiveMap(prefix, v, result, depth) + // map[string]any values are `any`, so this container can reference itself (a cycle) or be + // shared. Track its identity on the recursion stack and skip if we are already inside it. + ptr := reflect.ValueOf(v).Pointer() + if _, cyclic := seen[ptr]; cyclic { + log.Debug("openfeature: skipping attribute %q: cyclic evaluation context reference", prefix) + return + } + if seen == nil { + seen = make(map[uintptr]struct{}, 1) + } + seen[ptr] = struct{}{} + flattenRecursiveMap(prefix, v, result, seen, depth) + delete(seen, ptr) case map[string]string: - flattenRecursiveMap(prefix, v, result, depth) + flattenRecursiveMap(prefix, v, result, seen, depth) case map[string]uint: - flattenRecursiveMap(prefix, v, result, depth) + flattenRecursiveMap(prefix, v, result, seen, depth) case map[string]int: - flattenRecursiveMap(prefix, v, result, depth) + flattenRecursiveMap(prefix, v, result, seen, depth) case map[string]int64: - flattenRecursiveMap(prefix, v, result, depth) + flattenRecursiveMap(prefix, v, result, seen, depth) case map[string]int32: - flattenRecursiveMap(prefix, v, result, depth) + flattenRecursiveMap(prefix, v, result, seen, depth) case map[string]uint64: - flattenRecursiveMap(prefix, v, result, depth) + flattenRecursiveMap(prefix, v, result, seen, depth) case map[string]uint32: - flattenRecursiveMap(prefix, v, result, depth) + flattenRecursiveMap(prefix, v, result, seen, depth) case map[string]int16: - flattenRecursiveMap(prefix, v, result, depth) + flattenRecursiveMap(prefix, v, result, seen, depth) case map[string]int8: - flattenRecursiveMap(prefix, v, result, depth) + flattenRecursiveMap(prefix, v, result, seen, depth) case map[string]uint16: - flattenRecursiveMap(prefix, v, result, depth) + flattenRecursiveMap(prefix, v, result, seen, depth) case map[string]uint8: - flattenRecursiveMap(prefix, v, result, depth) + flattenRecursiveMap(prefix, v, result, seen, depth) case map[string]float64: - flattenRecursiveMap(prefix, v, result, depth) + flattenRecursiveMap(prefix, v, result, seen, depth) case map[string]bool: - flattenRecursiveMap(prefix, v, result, depth) + flattenRecursiveMap(prefix, v, result, seen, depth) case map[string]float32: - flattenRecursiveMap(prefix, v, result, depth) + flattenRecursiveMap(prefix, v, result, seen, depth) + case []any: + // []any elements are `any` and can likewise reference the slice itself or be shared. + ptr := reflect.ValueOf(v).Pointer() + if _, cyclic := seen[ptr]; cyclic { + log.Debug("openfeature: skipping attribute %q: cyclic evaluation context reference", prefix) + return + } + if seen == nil { + seen = make(map[uintptr]struct{}, 1) + } + seen[ptr] = struct{}{} + flattenRecursiveArray(prefix, v, result, seen, depth) + delete(seen, ptr) case []string: - flattenRecursiveArray(prefix, v, result, depth) + flattenRecursiveArray(prefix, v, result, seen, depth) case []int: - flattenRecursiveArray(prefix, v, result, depth) + flattenRecursiveArray(prefix, v, result, seen, depth) case []int64: - flattenRecursiveArray(prefix, v, result, depth) + flattenRecursiveArray(prefix, v, result, seen, depth) case []int32: - flattenRecursiveArray(prefix, v, result, depth) + flattenRecursiveArray(prefix, v, result, seen, depth) case []uint64: - flattenRecursiveArray(prefix, v, result, depth) + flattenRecursiveArray(prefix, v, result, seen, depth) case []uint32: - flattenRecursiveArray(prefix, v, result, depth) + flattenRecursiveArray(prefix, v, result, seen, depth) case []int16: - flattenRecursiveArray(prefix, v, result, depth) + flattenRecursiveArray(prefix, v, result, seen, depth) case []uint16: - flattenRecursiveArray(prefix, v, result, depth) + flattenRecursiveArray(prefix, v, result, seen, depth) case []float64: - flattenRecursiveArray(prefix, v, result, depth) + flattenRecursiveArray(prefix, v, result, seen, depth) case []bool: - flattenRecursiveArray(prefix, v, result, depth) + flattenRecursiveArray(prefix, v, result, seen, depth) case []float32: - flattenRecursiveArray(prefix, v, result, depth) - case []any: - flattenRecursiveArray(prefix, v, result, depth) + flattenRecursiveArray(prefix, v, result, seen, depth) case []byte: result[prefix] = string(v) case fmt.Stringer: @@ -113,20 +156,20 @@ func flattenRecursiveDepth(prefix string, value any, result map[string]any, dept } } -func flattenRecursiveMap[T any](prefix string, v map[string]T, result map[string]any, depth int) { +func flattenRecursiveMap[T any](prefix string, v map[string]T, result map[string]any, seen map[uintptr]struct{}, depth int) { for key, val := range v { newPrefix := key if prefix != "" { newPrefix = prefix + "." + key } - flattenRecursiveDepth(newPrefix, val, result, depth+1) + flattenRecursiveDepth(newPrefix, val, result, seen, depth+1) } } -func flattenRecursiveArray[T any](prefix string, v []T, result map[string]any, depth int) { +func flattenRecursiveArray[T any](prefix string, v []T, result map[string]any, seen map[uintptr]struct{}, depth int) { for i, item := range v { flattenKey := prefix + "." + strconv.Itoa(i) - flattenRecursiveDepth(flattenKey, item, result, depth+1) + flattenRecursiveDepth(flattenKey, item, result, seen, depth+1) } } diff --git a/openfeature/flatten_test.go b/openfeature/flatten_test.go index d67db210aad..bc97f350c53 100644 --- a/openfeature/flatten_test.go +++ b/openfeature/flatten_test.go @@ -12,78 +12,135 @@ import ( "github.com/stretchr/testify/assert" ) -// TestFlattenContextNested is a regression guard: legitimate nested context still flattens to -// dot notation exactly as before the depth cap was added. -func TestFlattenContextNested(t *testing.T) { - in := map[string]any{ - "user": map[string]any{"id": "123", "email": "a@b.com"}, - "plan": "pro", +// TestFlattenContext exercises both the normal flattening behavior and the traversal bounds +// (depth cap, cycle detection, and the total-field safety ceiling) that keep an +// attacker-influenceable evaluation context from exhausting the stack or memory. +// +// Cyclic / self-referential / deeply nested contexts cannot be expressed as map literals, so each +// case builds its input via a closure and verifies the result via a closure. Exact-output cases +// assert equality; the safety-bound cases assert the result is bounded (and, critically, that the +// call returns at all — an unbounded traversal would fatally overflow the Go stack or OOM the +// process rather than fail an assertion). +func TestFlattenContext(t *testing.T) { + tests := []struct { + name string + build func() map[string]any + verify func(t *testing.T, got map[string]any) + }{ + { + name: "nested maps flatten to dot notation", + build: func() map[string]any { + return map[string]any{ + "user": map[string]any{"id": "123", "email": "a@b.com"}, + "plan": "pro", + } + }, + verify: func(t *testing.T, got map[string]any) { + assert.Equal(t, map[string]any{ + "user.id": "123", + "user.email": "a@b.com", + "plan": "pro", + }, got) + }, + }, + { + name: "nested arrays flatten with indices", + build: func() map[string]any { + return map[string]any{"tags": []any{"a", "b"}} + }, + verify: func(t *testing.T, got map[string]any) { + assert.Equal(t, map[string]any{"tags.0": "a", "tags.1": "b"}, got) + }, + }, + { + name: "direct self-reference is skipped, siblings kept", + build: func() map[string]any { + m := map[string]any{"name": "leo"} + m["self"] = m // cycle: m -> m + return m + }, + verify: func(t *testing.T, got map[string]any) { + // The cyclic branch is dropped entirely; the scalar sibling survives. + assert.Equal(t, map[string]any{"name": "leo"}, got) + }, + }, + { + name: "indirect cycle a -> b -> a is broken", + build: func() map[string]any { + a := map[string]any{"leaf": "x"} + b := map[string]any{"a": a} + a["b"] = b // a -> b -> a + return map[string]any{"root": a} + }, + verify: func(t *testing.T, got map[string]any) { + assert.Equal(t, map[string]any{"root.leaf": "x"}, got) + }, + }, + { + name: "shared subtree (diamond) is not treated as a cycle", + build: func() map[string]any { + shared := map[string]any{"v": "1"} + return map[string]any{"x": shared, "y": shared} + }, + verify: func(t *testing.T, got map[string]any) { + // Both sibling branches must fully flatten — cycle detection only breaks + // references that are live on the current recursion stack. + assert.Equal(t, map[string]any{"x.v": "1", "y.v": "1"}, got) + }, + }, + { + name: "map nesting beyond the depth cap is dropped", + build: func() map[string]any { + deep := map[string]any{"leaf": "deep-value"} + for range maxContextDepth + 10 { + deep = map[string]any{"n": deep} + } + return deep + }, + verify: func(t *testing.T, got map[string]any) { + for k, v := range got { + assert.NotEqual(t, "deep-value", v, "leaf below the depth cap must be dropped") + assert.LessOrEqual(t, strings.Count(k, "."), maxContextDepth) + } + }, + }, + { + name: "array nesting beyond the depth cap is dropped", + build: func() map[string]any { + var deep any = "leaf-value" + for range maxContextDepth + 10 { + deep = []any{deep} + } + return map[string]any{"arr": deep} + }, + verify: func(t *testing.T, got map[string]any) { + for _, v := range got { + assert.NotEqual(t, "leaf-value", v, "leaf below the depth cap must be dropped") + } + }, + }, + { + name: "shared-subtree fan-out is bounded by the field ceiling", + build: func() map[string]any { + // Branching-2 nesting reusing the same child at each level would expand to ~2^20 + // leaves without the ceiling; depth (20) stays under maxContextDepth so the depth + // cap alone does not save us here — only maxFlattenFields does. + var cur any = map[string]any{"x": "v"} + for range 20 { + cur = map[string]any{"a": cur, "b": cur} + } + return map[string]any{"root": cur} + }, + verify: func(t *testing.T, got map[string]any) { + assert.NotEmpty(t, got) + assert.LessOrEqual(t, len(got), maxFlattenFields) + }, + }, } - got := flattenContext(in) - assert.Equal(t, map[string]any{ - "user.id": "123", - "user.email": "a@b.com", - "plan": "pro", - }, got) -} - -// TestFlattenContextSelfReferenceDoesNotCrash verifies that a directly self-referential context -// terminates instead of recursing forever. In Go a stack overflow is fatal (recover() cannot -// catch it), so without the depth cap this would crash the whole process rather than fail the -// assertion. The cyclic branch is bounded to maxContextDepth levels. -func TestFlattenContextSelfReferenceDoesNotCrash(t *testing.T) { - m := map[string]any{"name": "leo"} - m["self"] = m // cycle: m -> m - - got := flattenContext(m) - - assert.Equal(t, "leo", got["name"]) - for k := range got { - assert.LessOrEqual(t, strings.Count(k, "."), maxContextDepth) - } -} - -// TestFlattenContextIndirectCycleDoesNotCrash covers an a -> b -> a cycle across two maps. -func TestFlattenContextIndirectCycleDoesNotCrash(t *testing.T) { - a := map[string]any{"leaf": "x"} - b := map[string]any{"a": a} - a["b"] = b // a -> b -> a - - got := flattenContext(map[string]any{"root": a}) - - assert.Equal(t, "x", got["root.leaf"]) - for k := range got { - assert.LessOrEqual(t, strings.Count(k, "."), maxContextDepth) - } -} - -// TestFlattenContextDeepMapBounded verifies nesting beyond maxContextDepth is dropped rather -// than exhausting the stack. -func TestFlattenContextDeepMapBounded(t *testing.T) { - deep := map[string]any{"leaf": "deep-value"} - for range maxContextDepth + 10 { - deep = map[string]any{"n": deep} - } - - got := flattenContext(deep) - - // The leaf sits below the depth cap, so it must not appear, and no key may exceed the cap. - for k, v := range got { - assert.NotEqual(t, "deep-value", v) - assert.LessOrEqual(t, strings.Count(k, "."), maxContextDepth) - } -} - -// TestFlattenContextDeepArrayBounded is the array analogue of the deep-map bound. -func TestFlattenContextDeepArrayBounded(t *testing.T) { - var deep any = "leaf-value" - for range maxContextDepth + 10 { - deep = []any{deep} - } - - got := flattenContext(map[string]any{"arr": deep}) - for _, v := range got { - assert.NotEqual(t, "leaf-value", v) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tt.verify(t, flattenContext(tt.build())) + }) } }