google / google/etils

epath.Path.replace() silently destroys the destination file on Windows when the source is a directory

Open
#816 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
259
Forks
34
Avg merge
57m
Merged PRs (30d)
1

Description

`_OsPathBackend.replace()` guards only the destination:

```python
def replace(self, path: PathLike, dst: PathLike) -> None:
if self.isdir(dst):
raise IsADirectoryError(f'Cannot overwrite: {dst} is a directory')
os.replace(path, dst)
```

Nothing checks for *source is a directory, destination is an existing file*. On Windows `os.replace` accepts that, and the destination file's contents are gone.

```python
# Windows 11, CPython 3.13.13
os.mkdir(srcdir)
open(os.path.join(srcdir, 'payload.txt'), 'w').write('inside the source dir')
open(dst, 'w').write('IRREPLACEABLE DESTINATION CONTENT')

os.replace(srcdir, dst) # returns, no exception
os.path.isdir(dst) # True
os.listdir(dst) # ['payload.txt'] - original bytes gone
```

On POSIX `rename(2)` returns `ENOTDIR` itself, so the missing guard never mattered there, and `pytest_and_autopublish.yml` runs `ubuntu-latest` only.

The sibling `_FileSystemSpecBackend.replace()` already has the check at `backend.py:548-551`, so the two backends currently disagree about the same `Backend` contract.

`test_backend[_test_replace]` already asserts the correct behaviour and reports `SUBFAILED[os/dst-folder0-file.txt] - Failed: DID NOT RAISE NotADirectoryError`. It has never run on Windows, because the module fails collection there on `import grp`.

Two things I hit attempting a patch, which is why this is an issue and not a PR:

`exists` and `isdir` delegate straight to `os.path.exists` and `os.path.isdir` (`backend.py:135-139`), and both dereference symlinks where `rename(2)` does not. Copying the sibling's guard verbatim would raise for a symlink-to-directory source, which POSIX permits. Adding `not os.path.islink(path)` restores that, but Windows directory symlinks carry `FILE_ATTRIBUTE_DIRECTORY` on the link entry itself, so I could not establish the carve-out is safe on the platform the guard exists for. Creating a symlink here needs a privilege this account lacks (WinError 1314).

Separately, `backend.py:217-218` raises for any directory `dst`, while `rename(2)` succeeds onto an empty one.

I have no Linux or macOS machine, so every POSIX statement above is read from the contract rather than measured.

Contributor guide

Open the contributing guide

Research direction

Start in backend.py at _OsPathBackend.replace(), then compare its behavior with _FileSystemSpecBackend.replace() at lines 548-551. Run test_backend[_test_replace] on Windows after addressing the import grp collection failure, and verify the source-directory/destination-file case without breaking the documented symlink and destination-directory behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
operating-systems
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.