Key transforms silently leak: every non-dunder method of the cosmodol stores acts on the UNMAPPED key (four are destructive)

[thorwhalen]

Summary

Every public method of cosmodol's stores that takes a key — replace, batch, query, partition, add_container, add_database, delete — passes that key straight to the Cosmos SDK without applying whatever key transform the caller wrapped the store in. The __getitem__ / __setitem__ / __delitem__ / __contains__ / __iter__ path maps correctly; the named methods do not. Four of the nine cases destroy or overwrite data in the wrong place.

This is the cosmodol instance of the family-wide defect tracked in i2mint/dol#83 (umbrella), root cause i2mint/dol#18.

Status in this package: LATENT, not live. cosmodol never applies a key codec to its own stores — grep counts across the package are mk_relative_path_store 0, KeyCodecs 0, prefixless_view 0, filt_iter 0, PrefixRelativizationMixin 0, wrap_kvs 0 in code (only docstring mentions at cosmodol/stores.py:143 and cosmodol/recipes.py:57, plus misc/docs/). There is no _id_of_key anywhere. Every store class is a raw leaf. So nothing misbehaves out of the box — the hazard fires the moment a user layers a key codec on top, which misc/docs/architecture.md (Layer C) and recipes.cosmos_store(value_codec=...) actively invite them to do.

Mechanism

dol wraps stores by delegation (has-a), not inheritance. The wrapper holds the leaf in self.store. Two routes both hand the leaf's own bound method to the caller:

• Route A — instance-wraps. Store.__getattr__ (dol/base.py:742) returns getattr(self.store, attr).
• Route B — class-wraps. delegate_to (dol/base.py:416-480) installs a DelegatedAttribute for every attr in dir(wrapped); its __get__ (dol/base.py:279) also returns getattr(instance.store, attr).

Either way the method runs bound to the inner store and receives the outer key. Both routes were exercised below and behave identically.

There is a value-side mirror of the same defect: methods that touch values on the leaf bypass obj_of_data / data_of_obj. In cosmodol that shows up in partition() and query().

Why this package makes it especially easy to hit

Two aggravating specifics, both cosmodol-only:

1. The package steers users off the safe path onto the unsafe one. CosmosDatabase.__setitem__ (cosmodol/trees.py:93) and CosmosAccount.__setitem__ (cosmodol/trees.py:229) refuse and say "use add_container(...) / add_database(...)". CosmosDatabase.__delitem__ (cosmodol/trees.py:114) and CosmosAccount.__delitem__ (cosmodol/trees.py:242) refuse non-empty resources and say "call .delete(k, force=True)". The dunders map keys correctly; the methods they redirect to do not.
2. recipes.cosmos_store(value_codec=...) (cosmodol/recipes.py:112) is a documented, first-class parameter that returns a wrap_kvs-wrapped store. A user who takes that path and then calls .partition(pk) gets an unwrapped leaf back — no dol knowledge required to reach the bug.

Note also that Cosmos ids forbid / \ ? # (cosmodol/errors.py:79), so realistic namespacing uses a separator like app_; the repros below use that.

Affected symbols

Symbol	Location	Key arg	Verdict	Severity
CosmosAccount.delete(k, *, force=True)	cosmodol/trees.py:267	k (pos 1)	latent	destructive — drops an entire database plus every container in it
CosmosDatabase.delete(k, *, force=True)	cosmodol/trees.py:162	k (pos 1)	latent	destructive — deletes every document in, then drops, a container
CosmosItems.batch(operations)	cosmodol/stores.py:251	ids nested in operations	latent	destructive — transactional delete/replace/upsert at unmapped ids
CosmosItems.replace(k, v, *, etag)	cosmodol/stores.py:222	k (pos 1)	latent	destructive — full-document overwrite of the wrong item
CosmosPartitionedItems.partition(pk_value)	cosmodol/stores.py:444	pk_value (pos 1)	latent	wrong-scope — returns a store with the entire wrapper stack stripped
CosmosDatabase.add_container(name, ...)	cosmodol/trees.py:127	name (pos 1)	latent	wrong-scope — provisions a container outside the namespace
CosmosAccount.add_database(name, ...)	cosmodol/trees.py:257	name (pos 1)	latent	wrong-scope — provisions a database outside the namespace
CosmosItems.query(sql, ...)	cosmodol/stores.py:237	none (scope-wide)	latent	silent-wrong-result — yields leaf ids, bypasses value codecs
CosmosPartitionedItems.query(sql, ...)	cosmodol/stores.py:457	partition_key (kw)	latent	silent-wrong-result — same, plus unmapped partition_key

One correction to how this was first written up: replace does not create a phantom document. container.replace_item requires the target to exist, so the two real outcomes are a loud ItemNotFoundError (nothing at the bare id) or a silent full overwrite of a real, unrelated document (something is at the bare id). The second is the dangerous one and is what the repro shows.

Repro — item stores

Real repro, not synthetic: the classes under test are the real CosmosItems / CosmosPartitionedItems. Only the Cosmos backend is faked, using the package's own cosmodol.testing.FakeContainerProxy. Requires azure-cosmos importable; no Cosmos account and no network.

from dol import KeyCodecs
from cosmodol import CosmosItems
from cosmodol.testing import FakeContainerProxy

PK = "tenant-1"


def container():
    c = FakeContainerProxy(partition_key_path="/_pk")
    c.upsert_item({"id": "app_doc1", "_pk": PK, "payload": "MINE"})          # ours
    c.upsert_item({"id": "doc1", "_pk": PK, "payload": "SOMEONE ELSE'S"})    # not ours
    return c


def state(c):
    return {i: d["payload"] for (_pk, i), d in sorted(c._data.items())}


# 1. replace() writes to the UNMAPPED id ------------------------------------
c = container()
w = KeyCodecs.prefixed("app_")(CosmosItems)(          # class-wrap  (Route B)
    c, partition_key_value=PK, partition_key_path="/_pk"
)
assert w["doc1"]["id"] == "app_doc1"                  # __getitem__ maps correctly
w.replace("doc1", {"payload": "UPDATED"})
print(state(c))
# {'app_doc1': 'MINE', 'doc1': 'UPDATED'}
#   ^ ours, untouched          ^ unrelated document, replaced in full

# 2. identical on the other delegation route --------------------------------
c = container()
w = KeyCodecs.prefixed("app_")(                       # instance-wrap (Route A)
    CosmosItems(c, partition_key_value=PK, partition_key_path="/_pk")
)
w.replace("doc1", {"payload": "UPDATED"})
print(state(c))
# {'app_doc1': 'MINE', 'doc1': 'UPDATED'}

# 3. query() yields LEAF ids -------------------------------------------------
c = container()
w = KeyCodecs.prefixed("app_")(CosmosItems)(
    c, partition_key_value=PK, partition_key_path="/_pk"
)
print(list(w.query("SELECT VALUE c.id FROM c")))
# ['app_doc1', 'doc1']
#   -> w['app_doc1'] raises ItemNotFoundError('app_app_doc1')   (double-transform)
#   -> w['doc1']     silently returns the app_doc1 document     (wrong document)

partition() needs no key codec at all to misbehave — a plain value codec is enough:

from dol import wrap_kvs
from cosmodol import CosmosPartitionedItems

c = container()
Redacted = wrap_kvs(
    CosmosPartitionedItems, obj_of_data=lambda d: {**d, "payload": "<redacted>"}
)
p = Redacted(c, partition_key_path="/_pk", silent_full_scan=True)
print(p[(PK, "doc1")]["payload"])     # '<redacted>'   codec applied
sub = p.partition(PK)
print(type(sub).__name__)             # 'CosmosItems'  <-- unwrapped leaf
print(sub["doc1"]["payload"])         # "SOMEONE ELSE'S"   codec bypassed

Repro — destructive case (CosmosDatabase.delete(..., force=True))

cosmodol.testing has no fake DatabaseProxy, so the two backend objects here are hand-rolled. CosmosDatabase itself is the real class.

from azure.cosmos import DatabaseProxy
from dol import KeyCodecs
from cosmodol import CosmosDatabase


class FakeContainer:
    def __init__(self, cid, items):
        self.id, self.items = cid, list(items)

    def read(self):
        return {"id": self.id}

    def query_items(self, query, enable_cross_partition_query=False, **kw):
        yield from (dict(i) for i in self.items)

    def delete_item(self, item, partition_key=None):
        self.items = [i for i in self.items if i["id"] != item]


class FakeDatabase(DatabaseProxy):
    def __init__(self, containers):
        self.id, self.containers = "db", containers

    def read(self):
        return {"id": self.id}

    def list_containers(self):
        return [{"id": n} for n in self.containers]

    def get_container_client(self, k):
        return self.containers[k]

    def delete_container(self, k):
        print(f"DROPPED container {k!r} ({len(self.containers[k].items)} docs left)")
        del self.containers[k]


db = FakeDatabase(
    {
        "app_events": FakeContainer("app_events", [{"id": "e1"}]),
        "events": FakeContainer("events", [{"id": f"prod-{i}"} for i in range(3)]),
    }
)

view = KeyCodecs.prefixed("app_")(CosmosDatabase)(db)
assert view["events"].id == "app_events"    # the dunder maps correctly

view.delete("events", force=True)           # user means app_events
print(sorted(db.containers))

# DROPPED container 'events' (0 docs left)
# ['app_events']

Three production documents deleted one by one, then the container dropped — and the container the user actually meant is still there. CosmosAccount.delete(k, force=True) is the same shape one level up: it drops every container of the wrong database and then the database.

User-visible consequence

For a user who namespaces a shared Cosmos container/database with a key codec (the obvious multi-tenant pattern, and the one misc/docs/architecture.md recommends composing):

• store.replace(k, v) — permanently overwrites the entire body of a document belonging to a different namespace, if one happens to carry the bare id; otherwise raises ItemNotFoundError for a key that store[k] reads fine. Either way the intended document is never updated.
• store.batch([...]) — same, in a single Cosmos transaction, for every op in the batch, including delete.
• db_store.delete(name, force=True) — deletes every document in, and then drops, a container the caller does not own. Not recoverable without a Cosmos backup/restore.
• account_store.delete(name, force=True) — drops an entire database and every container in it. Worst case in the package.
• add_container / add_database — creates the resource outside the namespace, where the view cannot see it; with throughput= set, that is silent recurring spend on an orphan.
• query() — returns ids that are not valid keys of the store, some of which nevertheless resolve (to the wrong document) when fed back in; also returns rows from outside the namespace and skips any value codec.
• partition() — returns a store with the whole wrapper stack removed: key codec, value codec, everything. Reachable from cosmos_store(value_codec=...) without touching dol.

Suggested remediation

1. Resolve the leaf key inside each affected method. The escape hatch is:

from dol import wrapped_self          # exported from dol
from dol.dig import inner_most_key    # NOT exported from dol — import from dol.dig


def _leaf_key(self, k):
    """Map an outer key through the whole wrapper chain down to the leaf."""
    outer = wrapped_self(self)
    if outer is not self:
        inner = inner_most_key(outer, k)
        if isinstance(inner, str):     # see trap 2 below
            return inner
    return k

Two traps, both load-bearing:

• inner_most_key walks the whole chain, including the leaf's own _id_of_key. It replaces self._id_of_key(k); it must never be composed with it or the key is transformed twice. cosmodol has no _id_of_key, so there is nothing to compose with here — but keep it that way if one is ever added.
• It returns None silently when no layer in the chain defines _id_of_key, so a type check is mandatory. For CosmosItems a str check is right. For CosmosPartitionedItems keys are (pk_value, id) tuples, so a str check would reject every valid key — use a shape check (isinstance(inner, tuple) and len(inner) == 2) there instead.

Applied to replace, this was verified to fix both delegation routes and leave the unwrapped case byte-identical:

def replace(self, k, v, *, etag=None):
    k = _leaf_key(self, k)                     # replaces, not composes
    body = self._prepare_body(k, v)
    item, hdrs = point_replace(self.container, k, body, self.partition_key_value, etag=etag)
    self._observe("replace", hdrs)
    return self._maybe_strip(item)

# class-wrap : {'app_doc1': 'NEW',  'doc1': 'PRECIOUS'}   correct
# inst-wrap  : {'app_doc1': 'NEW2', 'doc1': 'PRECIOUS'}   correct
# unwrapped  : {'app_doc1': 'MINE', 'doc1': 'NEW3'}       unchanged behaviour

2. Fix partition() by re-wrapping, not by mapping a key. It returns a whole store, so key resolution alone does not help — the child must inherit the parent's transform stack. Either rebuild the child through wrapped_self(self)'s wrapper chain, or, if that is not practical, make partition() raise when wrapped_self(self) is not self rather than silently hand back an unwrapped leaf. Same reasoning applies to the store_factory seam in CosmosDatabase.

3. batch() cannot be fixed this way — the ids live inside opaque (op, args, kwargs) triples that no wrapper can see into. Either document it loudly as leaf-key-only, or give it a cosmodol-shaped operation type whose key field is explicit and therefore mappable.

4. Consider shrinking the surface instead. Most of these methods are per-object operations bolted onto a store. The alternative that removes the whole class of bug is the one azuredol took with BlobHandle: move per-object operations onto a handle whose key is resolved once at construction, e.g. store.item(k).replace(v) / db_store.container(name).drop(force=True). Then there is no second key-taking entry point to get wrong. Given that four of these methods exist only because __setitem__/__delitem__ were deliberately disabled as "too parameter-rich" (misc/docs/design_decisions.md §7), a handle is arguably the design that was wanted anyway.

5. Guard the destructive pair regardless. Until 1–4 land, CosmosDatabase.delete and CosmosAccount.delete should refuse to run with force=True when wrapped_self(self) is not self and no key resolution was applied. Cascading a delete onto an unverified name is not something to leave to a docstring.

---

Note on in-flight upstream fixes (added when filing)

Two dol PRs are open and change details referenced above:

• i2mint/dol#84 — inner_most_key and unravel_key
  become importable from dol directly (no more from dol.dig import ...), and
  inner_most_key now raises instead of returning None when no layer of the chain
  defines _id_of_key. If you write a local shim, the isinstance(_id, str) guard becomes
  unnecessary once that lands — but the "it replaces _id_of_key, never composes with it" trap
  still applies.
• i2mint/dol#85 — fixes dol.content_url to
  resolve the key through wrapping layers, and makes mk_relative_path_store install
  key-mapping is_valid_key/validate_key. Any finding above that is inherited from
  dol.filesys.Files is repaired by #85 with no change needed in this repo — this issue will
  be closed with verification once it merges.

Design context for why the ecosystem-wide answer is not "sprinkle wrapped_self everywhere":
i2mint/s3dol#14 and
s3dol ADR-0011.
Short version: wrapped_self is a best-effort guardrail with its own silent failure mode (it
degrades to the raw leaf when nothing holds a reference to the wrapper), so the durable fix is
to have no key-taking methods rather than to harden each one.