fix: refuse array changes that would drop element extensions

A changed array is replaced whole by the writer's value, which silently lost
any member a newer writer had added inside its elements. Encode now inspects
the array as read: if any element carries a member its typed schema does not
know, at any depth, it returns ErrRecordExtensionConflict naming the array and
the first such member. Arrays without such members are still replaced whole,
unchanged arrays are still kept verbatim, and arrays inside unknown members are
never inspected.

spinifex/domains/ec2/instance/state/record_codec.go

@@ -9,8 +9,10 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"maps"
 	"math"
 	"reflect"
+	"slices"
 	"strconv"
 	"strings"
 	"sync"
@@ -43,6 +45,7 @@ var (
 	ErrRecordFormatOutOfRange   = errors.New("instance record format is below 1")
 	ErrRecordFormatAhead        = errors.New("instance record format is newer than this binary can write")
 	ErrRecordSectionsNeedFormat = errors.New("assignment and observation need record format 2")
+	ErrRecordExtensionConflict  = errors.New("instance record change would drop an unknown member inside an array")
 )
 
 // AssignmentReason says why an assignment changed.
@@ -190,7 +193,7 @@ func DecodeTransitionRecord(data []byte) (*TransitionRecord, error) {
 
 // Encode writes only the typed paths that differ from the decoded snapshot
 // over the input, keeping unknown members. A changed array is replaced whole,
-// losing unknown members in its elements. It never adds a format marker.
+// and refused if its read elements carry unknown members. It adds no marker.
 func (r *TransitionRecord) Encode() ([]byte, error) {
 	if r.readOnly {
 		return nil, fmt.Errorf("%w: format %d", ErrRecordFormatAhead, r.format)
@@ -206,7 +209,11 @@ func (r *TransitionRecord) Encode() ([]byte, error) {
 	if raw == nil {
 		raw, snapshot = map[string]any{}, map[string]any{}
 	}
-	return json.Marshal(mergeChanged(transitionSchema(), raw, snapshot, current))
+	merged, err := mergeChanged(transitionSchema(), raw, snapshot, current, "$")
+	if err != nil {
+		return nil, err
+	}
+	return json.Marshal(merged)
 }
 
 // typedTree re-encodes the typed fields that the record's format covers.
@@ -238,15 +245,20 @@ func (r *TransitionRecord) typedTree() (map[string]any, error) {
 // mergeChanged applies the paths where current differs from snapshot onto
 // raw. A struct merges per member and keeps members it does not know; a map
 // merges per entry; any other differing value is replaced by current's.
-func mergeChanged(s *fieldSchema, raw, snapshot, current any) any {
+func mergeChanged(s *fieldSchema, raw, snapshot, current any, path string) (any, error) {
 	if reflect.DeepEqual(snapshot, current) {
-		return raw
+		return raw, nil
+	}
+	if rawArr, ok := raw.([]any); ok && s != nil {
+		if at, found := unknownMember(s, rawArr, path); found {
+			return nil, fmt.Errorf("%w: changed array %s would drop unknown member %s", ErrRecordExtensionConflict, path, at)
+		}
 	}
 	rawObj, rawIsObj := raw.(map[string]any)
 	snapObj, snapIsObj := snapshot.(map[string]any)
 	curObj, curIsObj := current.(map[string]any)
 	if s == nil || (s.fields == nil && !s.isMap) || !rawIsObj || !snapIsObj || !curIsObj {
-		return current
+		return current, nil
 	}
 	out := make(map[string]any, len(rawObj))
 	for k, v := range rawObj {
@@ -268,14 +280,49 @@ func mergeChanged(s *fieldSchema, raw, snapshot, current any) any {
 		}
 		switch {
 		case inSnap && inRaw:
-			out[k] = mergeChanged(child, rawV, snapV, curV)
+			merged, err := mergeChanged(child, rawV, snapV, curV, path+"."+k)
+			if err != nil {
+				return nil, err
+			}
+			out[k] = merged
 		case inSnap && !s.isMap && reflect.DeepEqual(snapV, curV):
 			// A zero member the input left out stays out.
 		default:
 			out[k] = curV
 		}
 	}
-	return out
+	return out, nil
+}
+
+// unknownMember finds the first member of v, at any depth, that schema s does
+// not know. An opaque value (nil schema) is not inspected.
+func unknownMember(s *fieldSchema, v any, path string) (string, bool) {
+	if s == nil {
+		return "", false
+	}
+	switch v := v.(type) {
+	case []any:
+		for i, e := range v {
+			if at, found := unknownMember(s.elem, e, path+"["+strconv.Itoa(i)+"]"); found {
+				return at, true
+			}
+		}
+	case map[string]any:
+		for _, k := range slices.Sorted(maps.Keys(v)) {
+			child := s.elem
+			if !s.isMap {
+				known, ok := s.fields[k]
+				if !ok {
+					return path + "." + k, true
+				}
+				child = known
+			}
+			if at, found := unknownMember(child, v[k], path+"."+k); found {
+				return at, true
+			}
+		}
+	}
+	return "", false
 }
 
 // recordFormat reads the marker: absent is 1, an integer above 2 is returned

spinifex/domains/ec2/instance/state/record_codec_test.go

@@ -230,8 +230,8 @@ func TestTransitionRecord_ExplicitMutationShadowsRawForm(t *testing.T) {
 }
 
 // An unchanged array is written back as read, unknown element members and
-// all. A changed array is the writer's value: unknown members inside its
-// elements are lost. That is a documented limitation, pinned here.
+// all. A changed array is replaced whole, unless a read element carries an
+// unknown member at any depth: then Encode refuses rather than drop it.
 func TestTransitionRecord_Arrays(t *testing.T) {
 	t.Run("unchanged array keeps unknown element members", func(t *testing.T) {
 		r := decode(t, goldenV1)
@@ -240,13 +240,59 @@ func TestTransitionRecord_Arrays(t *testing.T) {
 		refs, _ := member(t, out, "metadata", "owner_refs")
 		assert.JSONEq(t, `[{"kind":"cluster","name":"c-1","future_ref":"r-1"}]`, string(refs))
 	})
-	t.Run("changed array is replaced and loses unknown element members", func(t *testing.T) {
-		r := decode(t, goldenV1)
+	t.Run("changed array with known members only is replaced whole", func(t *testing.T) {
+		doc := `{"metadata":{"name":"i-1","owner_refs":[{"kind":"cluster","name":"c-1"}]},"future":[{"x":1}]}`
+		r := decode(t, doc)
 		r.Record.Metadata.OwnerRefs = append(r.Record.Metadata.OwnerRefs, resource.OwnerRef{Kind: "asg", Name: "a-1"})
-		out := encode(t, r)
-		refs, _ := member(t, out, "metadata", "owner_refs")
-		assert.JSONEq(t, `[{"kind":"cluster","name":"c-1"},{"kind":"asg","name":"a-1"}]`, string(refs))
+		want := edit(t, doc, map[string]any{"metadata.owner_refs": []any{
+			map[string]any{"kind": "cluster", "name": "c-1"},
+			map[string]any{"kind": "asg", "name": "a-1"},
+		}})
+		assert.Equal(t, want, canonical(t, encode(t, r)))
 	})
+	t.Run("changed array of scalars is replaced whole", func(t *testing.T) {
+		doc := `{"metadata":{"name":"i-1","finalizers":["eni","volume"]}}`
+		r := decode(t, doc)
+		r.Record.Metadata.Finalizers = []string{"volume"}
+		want := edit(t, doc, map[string]any{"metadata.finalizers": []any{"volume"}})
+		assert.Equal(t, want, canonical(t, encode(t, r)))
+	})
+
+	conflicts := []struct {
+		name, doc, array, member string
+		mutate                   func(*state.TransitionRecord)
+	}{
+		{
+			"unknown member in an element",
+			goldenV1,
+			"$.metadata.owner_refs",
+			"$.metadata.owner_refs[0].future_ref",
+			func(r *state.TransitionRecord) {
+				r.Record.Metadata.OwnerRefs = append(r.Record.Metadata.OwnerRefs, resource.OwnerRef{Kind: "asg", Name: "a-1"})
+			},
+		},
+		{
+			"unknown member one level deeper in an element",
+			`{"status":{"instance":{"NetworkInterfaces":[{"NetworkInterfaceId":"eni-1",` +
+				`"Attachment":{"DeviceIndex":0,"FutureAttach":"x"}}]}}}`,
+			"$.status.instance.NetworkInterfaces",
+			"$.status.instance.NetworkInterfaces[0].Attachment.FutureAttach",
+			func(r *state.TransitionRecord) {
+				r.Record.Status.Instance.NetworkInterfaces[0].NetworkInterfaceId = aws.String("eni-2")
+			},
+		},
+	}
+	for _, tc := range conflicts {
+		t.Run("changed array refused: "+tc.name, func(t *testing.T) {
+			r := decode(t, tc.doc)
+			tc.mutate(r)
+			out, err := r.Encode()
+			require.ErrorIs(t, err, state.ErrRecordExtensionConflict)
+			assert.Contains(t, err.Error(), "changed array "+tc.array+" ")
+			assert.Contains(t, err.Error(), tc.member)
+			assert.Nil(t, out)
+		})
+	}
 }
 
 func TestTransitionRecord_V2DecodesSectionsAndKeepsLegacyMirror(t *testing.T) {
@@ -451,6 +497,8 @@ func TestTransitionRecord_CapableWriterPreservesWhatPlainTypedRoundTripStrips(t
 	}
 }
 
+// An unmutated record cannot raise an extension conflict: that needs a
+// changed array, so every accepted, writable input must round-trip.
 func FuzzTransitionRecordRoundTrip(f *testing.F) {
 	for _, seed := range []string{
 		goldenV1, goldenV2,