Skip to content

[BUG]: CopyOnWrite containers lose or duplicate data in un-overridden inherited methods (serialization, deepcopy, operators) #152

Closed

Description

@araujof

Current Behavior

CopyOnWriteList and CopyOnWriteDict override only a subset of their base-class methods. Every un-overridden inherited method reads the parent list/dict storage directly — and that storage stays empty until _materialize() runs on the first write. So on an unmaterialized (i.e. read-only, the common case) CoW container, those methods silently operate on nothing.

This is the same root cause as #54 / #55 (CopyOnWriteDict.__eq__) and #135 / #136 (CopyOnWriteList.__eq__). Both of those fixed a single operator. This issue tracks the remaining surface.

Unifying property: all of these behave correctly after any write. The defect only affects containers that were never mutated.

Critical — silent data loss through serialization and copy

These are the reachable, high-impact cases. wrap_payload_for_isolation() is the real production path (ManagerBase._isolate_payload):

class P(PluginPayload):
    server_id: str
    items: list = Field(default_factory=list)

iso = wrap_payload_for_isolation(P(server_id="s", items=["a", "b", "c"]))

list(iso.items)              # ['a', 'b', 'c']          reads fine
iso.model_dump()             # {'server_id': 's', 'items': []}     <-- data gone
iso.model_dump_json()        # {"server_id":"s","items":[]}        <-- data gone
copy.deepcopy(iso).items     # ['a','b','c','a','b','c']           <-- DUPLICATED
iso.model_copy(deep=True)    # ['a','b','c','a','b','c']           <-- DUPLICATED

RootModel payloads are affected regardless of policy, because _isolate_payload isolates them unconditionally (isinstance(effective_payload, RootModel)):

iso = wrap_payload_for_isolation(HttpHeaderPayload({"authorization": "Bearer x", "x-tenant": "acme"}))
dict(iso.root.items())   # {'authorization': 'Bearer x', 'x-tenant': 'acme'}
iso.model_dump()         # {}      <-- every header stripped

This matters because external plugins receive the payload via model_dump():

  • cpex/framework/external/grpc/client.py:228 — json_format.ParseDict(payload.model_dump(), payload_struct)
  • cpex/framework/external/unix/client.py:303-304 — same
  • cpex/framework/external/mcp/server/server.py:229

Call path: _isolate_payload() (manager.py:405, manager.py:692) → execute_plugin() → external client → payload.model_dump(). An out-of-process plugin would therefore see items: [] / args: {} / no headers, while an in-process plugin reading the same payload sees the real contents.

Note on scope: I confirmed the serialization/copy behavior directly and traced the call path by reading the code, but did not execute a live external-plugin round-trip. Worth confirming before assigning final severity — it's possible something downstream re-materializes. _safe_deepcopy(CopyOnWriteList([...])) also duplicates, which is the same mechanism.

Root cause of each: model_dump and copy/deepcopy both bypass the overridden accessors. Pydantic and json.dumps read the C-level base storage (empty → []/{}); copy reconstructs via __reduce_ex__, which yields the logical items and carries _original over in __dict__, so extend on the new object materializes 3 items and then appends 3 more.

json.dumps differs between the two types — CopyOnWriteDict → {}, but CopyOnWriteList happens to serialize correctly because the list encoder iterates.

Moderate — operators and lookups on CopyOnWriteList

With CopyOnWriteList(["a", "b", "c"]), unmaterialized:

Expression Actual Expected
cow < ["a"] True False
cow > ["a"] False True
cow <= [] True False
cow + ["d"] ["d"] ["a","b","c","d"]
cow * 2 [] 6 items
cow += ["d"] no-op (["a","b","c"]) ["a","b","c","d"]
cow *= 2 no-op (3 items) 6 items
cow.index("b") ValueError 1
cow.count("a") 0 1
list(reversed(cow)) [] ["c","b","a"]

cow += [...] silently discarding an append is the nastiest of these for plugin authors: it looks like a mutation and materializes nothing.

Moderate — CopyOnWriteDict

Expression Actual Expected
cow | {} {} {"a": 1, "b": 2}
cow.popitem() KeyError a (key, value) pair
list(reversed(cow)) [] ["b", "a"]
json.dumps(cow) {} {"a": 1, "b": 2}

Already correct on both types: __getitem__ (incl. slices), __len__, __iter__, __contains__, bool(), copy(), keys()/values()/items(), and all mutators that call _materialize(). Double-wrapping (CopyOnWriteList(CopyOnWriteList([...]))) is also safe.

Expected Behavior

Every read on a CoW container reflects the logical sequence/mapping, whether or not it has been materialized — and serializing or copying an isolated payload preserves its contents exactly.

Steps to Reproduce

import copy, json
from cpex.framework.memory import CopyOnWriteList, CopyOnWriteDict

cow = CopyOnWriteList(["a", "b", "c"])
assert list(cow) == ["a", "b", "c"]        # passes
assert cow.count("a") == 1                 # FAILS: 0
assert cow + ["d"] == ["a","b","c","d"]    # FAILS: ["d"]
assert copy.deepcopy(cow) == ["a","b","c"] # FAILS: duplicated

d = CopyOnWriteDict({"a": 1})
assert json.dumps(d) == '{"a": 1}'         # FAILS: {}

For the isolation path, see the wrap_payload_for_isolation / HttpHeaderPayload snippets above.

Logs / Error Output

Not applicable — every failure mode here is silent. No exception, no log line. That is the main hazard: a stripped payload is indistinguishable from a legitimately empty one, which is exactly how #135 stayed hidden in production.

Environment

Additional Context

Patching operators one at a time is what left this surface open twice already (#55, then #136). Two structural options:

  1. Route every read through _source() and override the full list/dict API, including __reduce_ex__/__deepcopy__ (for copy) and whatever hook Pydantic serialization needs. Keeps CoW laziness; broad surface to get right and to keep right as the base types gain methods.
  2. Materialize eagerly on any un-overridden access — e.g. implement __getattr__/__reduce_ex__ to _materialize() first. Gives up some laziness on unusual access patterns but makes the class correct by construction, which is the property the last two fixes each failed to deliver.

Option 2 seems the better trade for a security-relevant container: the failure mode of the current design is silent data loss on an authorization path, and read-only performance only matters if it is also correct. Whichever route, a test that asserts the logical contents survive model_dump(), model_dump_json(), deepcopy(), and model_copy(deep=True) would have caught the whole class.

Related: #54, #55 (CopyOnWriteDict.__eq__), #135, #136 (CopyOnWriteList.__eq__).

Activity

  1. self-assigned this
    on Aug 4, 2026
  2. moved this from Backlog to In progress in CPEXon Aug 4, 2026
  3. added this to the 0.1.3 milestone on Aug 4, 2026
  4. added theissue type on Aug 4, 2026
  5. added a commit that references this issue on Aug 6, 2026
    f4b4e0f
  6. moved this from In progress to Done in CPEXon Aug 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't workingtriage

Type

Projects

  • Status
    Done

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions