OSAccess: directory rename corruption and other filesystem edge-case bugs

OSAccess has additional filesystem bugs beyond the stale file path/name fixed by #1065.

Reproduction setup

Run each example independently with:

from pathlib import PurePosixPath as P
from pydantic_monty import MemoryFile, OSAccess

These findings were reproduced against os_access.py at commit 03f566bb using direct Python method calls. The test harness loaded the source with stubbed native imports (NOT_HANDLED and an unused MontyFileHandle class); none of these examples uses file handles. They have not been tested end-to-end through a Monty worker. Rename behavior and reads through a file were also compared with CPython on macOS.

1. Renaming a directory into its descendant corrupts the tree

fs = OSAccess([MemoryFile('/a/sub/f', 'x')])
fs.path_rename(P('/a'), P('/a/sub/moved'))

Actual: raises RecursionError. Afterwards, fs.path_exists(P('/a')) is false and the file object's path contains repeated sub/moved components. path_rename removes the source and inserts it into its own descendant before _update_paths_recursive walks the resulting cycle.

Expected: reject the rename before mutation, as CPython does with OSError: [Errno 22] Invalid argument, leaving the source intact.

2. Constructor conflict detection depends on input order

fs = OSAccess([
    MemoryFile('/a/child', 'x'),
    MemoryFile('/a', 'y'),
])
assert fs.path_is_file(P('/a'))
assert not fs.path_exists(P('/a/child'))

Actual: initialization succeeds and silently replaces the directory with a file, hiding the child. Reversing the input order raises ValueError for a file nested within another file.

Expected: reject the conflicting paths in either order.

3. Renaming a path to itself is not a no-op

File case:

file = MemoryFile('/a', 'x')
fs = OSAccess([file])
fs.path_rename(P('/a'), P('/a'))
assert file.deleted  # actual, even though /a still exists

Nonempty directory case:

fs = OSAccess([MemoryFile('/a/f', 'x')])
fs.path_rename(P('/a'), P('/a'))

Actual: raises OSError: [Errno 66] Directory not empty. Expected: both operations succeed without changing the file or directory, as CPython does. The file case calls delete() on the source because it is also the target.

4. Parent components are not resolved

fs = OSAccess([MemoryFile('/a/f', 'x')])
assert not fs.path_exists(P('/a/../a/f'))  # actual
assert fs.path_resolve(P('/a/../a/f')) == '/a/../a/f'  # actual

Expected: the existing file is found and resolve() returns /a/f. PurePosixPath does not collapse ..; the lookup treats it as a literal child name. This example concerns direct OSAccess calls. Correction: OsFunctionCall::to_args() lexically normalizes paths before delivering them to Python callbacks, so worker-driven calls do not preserve these components.

5. Wrong exception when an intermediate component is a file

fs = OSAccess([MemoryFile('/a', 'x')])
fs.path_read_text(P('/a/child'))

Actual: FileNotFoundError. Expected: NotADirectoryError, matching CPython. _get_entry returns None for both missing components and intermediate files.

6. Removing the empty root leaks a KeyError

fs = OSAccess()
fs.path_rmdir(P('/'))

Actual: KeyError('') from deleting the root path's empty basename. Expected: a filesystem error that refuses removal of the virtual root.

7. Special permission bits omit the file type in stat results

fs = OSAccess([MemoryFile('/a', 'x', permissions=0o4755)])
assert fs.path_stat(P('/a')).st_mode == 0o4755  # actual

Expected: st_mode includes stat.S_IFREG as well as the supplied permission bits. StatResult.file_stat adds the file type only when mode < 0o1000, which excludes permission modes containing setuid/setgid/sticky bits. StatResult.dir_stat uses the same threshold.

The changes in #1065 do not address these cases.

Additional findings from a broader review

The checks below used the locally built native Python bindings and a real Monty worker. The checkout was based on 03f566bb with uncommitted fixes for findings 1–7 above and the stale rename metadata from #1065. Those fixes passed 220 tests across test_os_access.py and test_os_access_raw.py; the additional failures below remained. The initial findings above were recorded before those local fixes.

8. Deleted file payloads accumulate in host memory (high priority)

from pydantic_monty import Monty

fs = OSAccess()
code = """
from pathlib import Path
for i in range(200):
    Path('/f').write_text('x' * 65536)
    Path('/f').unlink()
'finished'
"""
with Monty() as pool:
    with pool.checkout(limits={'max_memory': 4 * 1024 * 1024}) as session:
        assert session.feed_run(code, os=fs) == 'finished'

assert len(fs.files) == 200
assert all(file.deleted for file in fs.files)
assert sum(len(file.content) for file in fs.files) == 13_107_200

Actual: no file remains in the virtual tree, but self.files retains all 200 deleted files and 12.5 MiB of payload. Every new file is appended to that list; unlink marks it deleted without releasing its contents. The same retention applies to files overwritten by rename.

Host callbacks allocate outside the worker's max_memory limit, so this is not evidence that the worker allocator limit failed. It demonstrates that the built-in OSAccess implementation has no aggregate storage budget and retains historical payloads. Reusing an instance allows the accumulation to continue across feeds.

Expected: an explicit host storage budget and a defined retention policy for deleted/overwritten files. Any change must account for callers retaining MemoryFile objects and inspecting files or deleted.

9. Renaming a deep directory moves the tree before failing (high priority)

from pydantic_monty import Monty

old_path = '/a/' + '/'.join(['d'] * 1100) + '/f'
file = MemoryFile(old_path, 'x')
fs = OSAccess([file])
with Monty() as pool:
    with pool.checkout() as session:
        session.feed_run("from pathlib import Path\nPath('/a').rename('/b')", os=fs)

Actual: MontyRuntimeError: RecursionError: maximum recursion depth exceeded. After the failure:

assert not fs.path_exists(P('/a'))
assert fs.path_exists(P(old_path.replace('/a/', '/b/', 1)))
assert file.path == P(old_path)

This is an acyclic tree with a roughly 2.2 KiB path. The rename has already moved the directory when recursive metadata rewriting exceeds Python's recursion limit. Files are reachable under the new tree while their stored paths still point at the old location.

Expected: iterative traversal and validation/preparation before committing the move, so failure cannot leave tree keys and metadata inconsistent.

10. Text append decodes existing bytes unnecessarily

from pydantic_monty import Monty

fs = OSAccess([MemoryFile('/a', b'\xff')])
with Monty() as pool:
    with pool.checkout() as session:
        session.feed_run("""
with open('/a', 'a') as f:
    f.write('x')
open('/a', 'rb').read()
""", os=fs)

Actual: UnicodeDecodeError, because _append decodes the existing byte content before appending text. CPython appends successfully and leaves b'\xffx'; appending does not require decoding the existing bytes. The comments on path_append_text also say bytes-backed files receive appended bytes, contrary to the implementation.

Expected: encode the new text and append to byte-backed content without decoding the old bytes.

11. Modification timestamps change on every stat

from pydantic_monty import Monty

fs = OSAccess([MemoryFile('/a', 'x')])
with Monty() as pool:
    with pool.checkout() as session:
        result = session.feed_run("""
from pathlib import Path
p = Path('/a')
p.stat().st_mtime == p.stat().st_mtime
""", os=fs)
assert result is False  # actual, despite no intervening writes

path_stat constructs a new StatResult with the current time on every call. This affects files and directories and breaks modification detection/caching.

Expected: retain metadata timestamps and update them when the corresponding filesystem operations occur.

12. The proposed parent-component fix needs direct/worker API parity

This is a problem in the local uncommitted fix for finding 4, rather than an additional claim about unchanged main.

fs = OSAccess()
fs.path_mkdir(P('/new/../target'), parents=True, exist_ok=False)

Actual with that fix: FileNotFoundError for /new before creating anything. The equivalent snippet through a worker succeeds and creates only /target, because callback arguments are already lexically normalized. CPython creates both new and target.

The direct API and callback path need a deliberate, consistent normalization contract. The earlier claim in finding 4 that Python callbacks receive unnormalized components has been corrected above.

13. Path.read_text newline translation differs from open (shared with Rust mounts)

With MemoryFile('/a', b'a\r\nb\rc'):

Path('/a').read_text()  # actual: 'a\r\nb\rc'
open('/a').read()      # actual: 'a\nb\nc'

Both results were confirmed through a worker. CPython translates universal newlines in both cases by default. The same Path.read_text() result was reproduced using a Rust-backed MountDir, so this finding extends beyond OSAccess.

Additional security review

These checks used the same local checkout and real worker bindings described above.

14. Live-file growth and repeated append can exhaust host resources

The storage-budget gap in finding 8 also affects live files, without unlinking or retaining historical versions:

from pydantic_monty import Monty

fs = OSAccess([MemoryFile('/f', '')])
code = """
from pathlib import Path
chunk = 'x' * 65536
for i in range(100):
    with Path('/f').open('a') as f:
        f.write(chunk)
'finished'
"""
with Monty() as pool:
    with pool.checkout(limits={'max_memory': 1024 * 1024}) as session:
        assert session.feed_run(code, os=fs) == 'finished'

assert len(fs.files[0].content) == 6_553_600

Confirmed: a 1 MiB worker memory limit permits a 6.25 MiB host-side file after 100 appends. The worker sends a small fixed-size chunk on each call; the host retains the growing result. The default 1,000-suspension budget limits the number of calls but does not provide a byte budget. Additional sessions/feeds can continue growing a reused filesystem instance.

Each append concatenates the entire existing immutable string/bytes buffer. For fixed-size chunks, total copying is quadratic in the number of appends, and old/new buffers coexist transiently. The same pattern also affects byte-backed files and callback-backed append, which performs read/concatenate/write in the host.

Impact: untrusted code can consume host memory and CPU while staying below the worker's memory limit. This is a host-side availability issue; it does not demonstrate failure of the worker allocator limit. Worker execution-duration budgets exclude host callback time.

Required mitigation: an aggregate retained-storage limit, a cumulative write budget, and bounded append construction. Budget failures should occur before changing filesystem state. These limits should cover live files, deleted/overwritten payload retention, and transient copies.

15. Rename changes the path supplied to CallbackFile callbacks

from pydantic_monty import CallbackFile, Monty

observed_paths = []
file = CallbackFile(
    '/fixed',
    read=lambda path: '',
    write=lambda path, content: observed_paths.append(path),
)
fs = OSAccess([file])
with Monty() as pool:
    with pool.checkout() as session:
        session.feed_run("""
from pathlib import Path
Path('/fixed').rename('/controlled')
Path('/controlled').write_text('x')
""", os=fs)

assert observed_paths == [P('/controlled')]

Confirmed: sandbox code can change the callback's path argument through a virtual rename. Directory renames also rewrite contained file paths. Registering a callback at a fixed path therefore does not pin future callback arguments to that path.

This is an API misuse hazard, not a demonstrated sandbox escape. Callbacks already execute trusted host code with the host's privileges. A callback that uses the supplied path as a host filename, storage key, or authorization identity must reauthorize it on every call. The constructor's original registration path cannot serve as that authorization check. The warning should explicitly explain rename behavior; an immutable callback identity or an option to forbid callback-file moves would make this easier to use safely. Callbacks intentionally granting broad access remain the application's responsibility.

Security checks without additional findings

• A deterministic 10,000-operation test exercised create, append, mkdir, open, rename, unlink, and rmdir. After successful operations and expected failures, it checked directory cycles, file path/name consistency, deleted-file reachability, duplicate reachable file identities, and membership in the retained file list. No further tree-invariant failure was found in that bounded shallow-tree test. This does not cover the known deep-rename failure.
• Embedded-NUL creation through the worker was rejected before the OS handler ran.
• Inspection of the built-in MemoryFile filesystem operations found no host filesystem I/O. Paths remain virtual; host filesystem access requires a custom callback or another explicitly exposed capability.
• Shared OSAccess instances preserve state across separate checkouts, as expected for a mutable host object. They must not be reused between tenants that require isolated filesystem state.
• No host filesystem escape or memory-safety violation was demonstrated by these checks.

Scope and priorities

Address host storage retention/budgeting and deep-rename consistency first. The review did not treat documented path-based file handles, unsupported update modes, or optional-hook NotImplementedError fallback as new bugs. Embedded-NUL creation was rejected before reaching the handler. These checks found no host filesystem escape; they are not a proof that all security properties hold.