Key-transform bypass: aget gets the OUTER key — RelPathAioFileBytesReader.aget('a.txt') silently reads a file outside the store #1

Summary

AioFileBytesReader.aget — the only read method this package offers — is handed the outer, untransformed key when the class is wrapped in mk_relative_path_store. Since the package ships exactly such a wrapper (RelPathAioFileBytesReader), this is live today, and it is silent: aget resolves the relative key against the process working directory, so it can return the bytes of a completely unrelated file instead of the one in the store.

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

Mechanism

dol wraps stores by delegation (has-a), not inheritance. The wrapper maps keys for the dunder protocol (__getitem__ / __setitem__ / __delitem__ / __contains__ / __iter__) — and for nothing else. Every other non-dunder method reaches the leaf bound to the leaf, with the outer key unmapped. Two routes:

• Route A — instance-wraps and mk_relative_path_store subclasses. Store.__getattr__ (dol/base.py:742) returns getattr(self.store, attr) — the leaf-bound method.
• Route B — class-wraps. delegate_to (dol/base.py:416) installs a DelegatedAttribute per attr; its __get__ (dol/base.py:279) also returns getattr(instance.store, attr).

This package hits Route A. mk_relative_path_store builds type(store_cls.__name__, (PrefixRelativizationMixin, Store), {}) (dol/paths.py:1163) — a delegating Store, not a subclass of the leaf. So RelPathAioFileBytesReader(...).aget is AioFileBytesReader.aget bound to the inner store:

s.aget.__self__ is s.store   # True
s.aget.__qualname__          # 'AioFileBytesReader.aget'

aget then does AIOFile(k, ...) (aiofiledol/__init__.py:49) on a key like 'greeting' — a relative path, resolved by the OS against the current working directory.

Why it is silent rather than loud here

Three things line up badly:

1. __getitem__ = None (aiofiledol/__init__.py:28). The mapped read route is deliberately disabled — s['greeting'] raises TypeError: 'NoneType' object is not callable. aget is not a convenience alongside a correct __getitem__; it is the entire read API, and there is no working alternative to fall back on.
2. The key validator is commented out on aget (aiofiledol/__init__.py:30). asetitem keeps @validate_key_and_raise_key_error_on_exception (:84) and therefore fails loudly; aget has no such backstop. (The README still advertises KeyValidationError for an out-of-store key — in current code that call raises FileNotFoundError.)
3. The scoping the leaf does have is absolute-path based. is_valid_key matches self._key_pattern, built from the absolute rootdir — so it also returns the wrong answer for outer keys.

Net effect: aget reads whatever the relative path happens to name in the CWD, or raises FileNotFoundError for a key that list(s) just yielded and k in s just accepted.

Affected symbols

Not affected, verified: keys / items / values resolve on the wrapper and map correctly; with_relative_paths returns a correct single-level view here; the mapped __setitem__ route (Store.__setitem__ → leaf __setitem__ at :109 → asetitem with the already-mapped key) writes to the right place when run inside an event loop.

Reproduction (REAL — this package, run as-is)

import asyncio, os, tempfile
from aiofiledol import RelPathAioFileBytesReader

# a real store with a real file in it
root = tempfile.mkdtemp()
with open(os.path.join(root, 'greeting'), 'wb') as f:
    f.write(b'REAL STORE CONTENT')

# an unrelated file with the same *relative* name, in the process CWD
cwd = tempfile.mkdtemp()
os.chdir(cwd)
with open('greeting', 'wb') as f:
    f.write(b'DECOY FROM CWD')

s = RelPathAioFileBytesReader(root)

print(list(s))            # ['greeting']          the key is relative
print('greeting' in s)    # True                  the store agrees it exists

s['greeting']             # TypeError: 'NoneType' object is not callable   (__getitem__ = None)

print(asyncio.run(s.aget('greeting')))
# -> b'DECOY FROM CWD'    WRONG. Expected b'REAL STORE CONTENT'

os.remove('greeting')     # remove the decoy
asyncio.run(s.aget('greeting'))
# -> FileNotFoundError: [Errno 2] No such file or directory: 'greeting'
#    ...for a key that list(s) yields and `in` accepts.

print(s.is_valid_key('greeting'))   # False   WRONG

User-visible consequence

For any user of RelPathAioFileBytesReader / RelPathFileStringReader — the relative-path classes, i.e. the ergonomic ones:

• Reads silently return the wrong bytes. If a file with the same relative name exists under the process CWD, aget returns that file's contents. The store's rootdir is not consulted at all. A store that is supposed to be confined to rootdir reads arbitrary CWD-relative paths — including ../ traversals if keys are ever user-influenced.
• Otherwise reads fail on keys the store says it has. list(s), k in s, len(s) all work; s.aget(k) on the very key just enumerated raises FileNotFoundError. There is no fallback, because __getitem__ is None.
• Key validation lies in the other direction. s.is_valid_key(k) is False and s.validate_key(k) raises for every key the store legitimately contains, so user code that guards with these rejects everything.

No data is destroyed. The write path (asetitem) is latent and fail-loud: the retained validator rejects the unmapped key with KeyValidationError before any file is opened, so nothing is written to a wrong location and nothing is overwritten. This is a read-side confidentiality/correctness bug, not a destructive one.

Suggested remediation

Resolve the key through the whole wrapper chain at the top of each async method. wrapped_self climbs the back-reference registry from the leaf to the outermost wrapper (verified: wrapped_self(s.store) is s), and inner_most_key then walks the chain's _id_of_keys:

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

def _resolve_key(self, k):
    kk = inner_most_key(wrapped_self(self), k)
    return kk if isinstance(kk, str) else k   # str-check is mandatory, see below

class AioFileBytesReader(FileCollection, KvReader):
    async def aget(self, k):
        async with AIOFile(_resolve_key(self, k), **self._read_open_kwargs) as fp:
            return await fp.read()

Two traps, both real:

• inner_most_key walks the ENTIRE chain, including the leaf's own _id_of_key. It replaces self._id_of_key(k) and must never be composed with it, or the key is transformed twice.
• It returns None, silently, when no layer in the chain defines _id_of_key — which is exactly the un-wrapped case (inner_most_key(wrapped_self(bare_leaf), 'a.txt') → None). The isinstance(kk, str) fallback is not optional; without it, an un-relativized AioFileBytesReader would start passing None to AIOFile.

Verified working, both ways round:

# with the fix applied to aget:
RelFixed(root).aget('a.txt')                       # -> b'HI'   (relativized: correct file)
Fixed(root, max_levels=0).aget(f'{root}/a.txt')    # -> b'HI'   (bare: absolute keys still work)

Apply the same treatment to asetitem (:85) to close the latent write-path case. There is precedent for this pattern inside dol itself — dol/filesys.py:766 and :825 already use inner_most_key to recover the full path.

Two smaller items worth folding in while touching this file:

• Restore the @validate_key_and_raise_key_error_on_exception decorator on aget (currently commented out at :30). Once the key is resolved correctly, the validator turns any remaining mismatch into a loud KeyValidationError instead of a wrong read — and it makes the code match the README, which already documents that behaviour.
• __getitem__ = None (:28) removes every escape route. If a synchronous read cannot be supported, consider raising a NotImplementedError with a message pointing at aget, rather than a bare TypeError: 'NoneType' object is not callable.

Notes on the original survey

Two corrections, for the record: mk_relative_path_store appears at 2 call sites, not 3 (the third grep hit is the import at :10); and both call sites wrap readers, which is exactly why the read path is confirmed-live while the write path is only latent.

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.