Skip to content

Commit 09a8ed5

Browse files
fix: eliminate TOCTOU races in catalog_fetch() for file:// and bare path URLs
Remove exists() pre-checks and catch FileNotFoundError from read_text() for both file:// and bare path catalog sources. Also catches OSError/UnicodeError to preserve the decode-error wrapping contract. Add regression test for the TOCTOU fix covering both file:// URLs and bare paths: mocked Path is observable as present (exists() returns True) but read_text() raises FileNotFoundError, proving the exists() removal eliminates the race window. Co-authored-by: GitHub Copilot (model: mimo-v2.5-free, supervised)
1 parent e80c541 commit 09a8ed5

2 files changed

Lines changed: 29 additions & 0 deletions

File tree

src/specify_cli/bundler/services/adapters.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,13 +147,17 @@ def fetch(source: CatalogSource) -> dict:
147147
return loads_json(path.read_text(encoding="utf-8"), origin=str(path))
148148
except FileNotFoundError:
149149
raise BundlerError(f"Catalog file not found: {path}") from None
150+
except (OSError, UnicodeError) as exc:
151+
raise BundlerError(f"Could not read {path}: {exc}") from exc
150152

151153
if scheme == "" or _is_windows_drive_path(url):
152154
path = Path(url)
153155
try:
154156
return loads_json(path.read_text(encoding="utf-8"), origin=str(path))
155157
except FileNotFoundError:
156158
raise BundlerError(f"Catalog file not found: {path}") from None
159+
except (OSError, UnicodeError) as exc:
160+
raise BundlerError(f"Could not read {path}: {exc}") from exc
157161

158162
if scheme in ("http", "https"):
159163
if not allow_network:

tests/unit/test_bundler_adapters.py

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
"""Unit tests for catalog-fetch adapters (auth + redirect safety)."""
22
from __future__ import annotations
33

4+
from pathlib import Path
5+
from unittest.mock import MagicMock, patch
6+
47
import pytest
58

69
from specify_cli.bundler import BundlerError
@@ -201,3 +204,25 @@ def test_validate_remote_url_rejects_malformed_url_cleanly(url):
201204
caller. Bundler sibling of #3369."""
202205
with pytest.raises(BundlerError):
203206
adapters._validate_remote_url("team", url)
207+
208+
209+
@pytest.mark.parametrize("use_file_url", [False, True], ids=["path", "file-url"])
210+
def test_local_catalog_toctou_race(tmp_path, use_file_url):
211+
"""Regression guard: a file that disappears between the old exists() pre-check
212+
and read_text() must raise BundlerError, not a raw FileNotFoundError.
213+
214+
The mocked Path is observable as present (exists() returns True) but
215+
read_text() raises FileNotFoundError, simulating a deletion between the two
216+
calls — the exact race window the exists() removal eliminates."""
217+
catalog_path = tmp_path / "catalog.json"
218+
url = catalog_path.as_uri() if use_file_url else str(catalog_path)
219+
220+
mock_path = MagicMock(spec=Path)
221+
mock_path.exists.return_value = True
222+
mock_path.read_text.side_effect = FileNotFoundError(str(catalog_path))
223+
224+
fetcher = adapters.make_catalog_fetcher(allow_network=False)
225+
226+
with patch.object(adapters.Path, "__new__", return_value=mock_path):
227+
with pytest.raises(BundlerError, match="Catalog file not found"):
228+
fetcher(_source(url))

0 commit comments

Comments
 (0)