diff --git a/strix/interface/cloud/source_upload.py b/strix/interface/cloud/source_upload.py index 780a8456..0c7e93ea 100644 --- a/strix/interface/cloud/source_upload.py +++ b/strix/interface/cloud/source_upload.py @@ -585,6 +585,21 @@ def _matches_user_pattern(relative: Path, pattern: str) -> bool: return posix.match(pattern) or fnmatch.fnmatch(relative_posix, pattern) +def _is_stat_unchanged(stat_result: os.stat_result, item: SelectedFile) -> bool: + if ( + not stat.S_ISREG(stat_result.st_mode) + or stat_result.st_size != item.size + or stat_result.st_dev != item.device + or stat_result.st_ino != item.inode + or stat_result.st_mtime_ns != item.mtime_ns + ): + return False + # On Windows (NTFS/FAT), st_ctime represents file creation time rather than + # POSIX inode change time, and its nanosecond timestamp can settle asynchronously + # after creation without any content or attribute changes. + return os.name == "nt" or stat_result.st_ctime_ns == item.ctime_ns + + def _write_archive(destination: Path, files: tuple[SelectedFile, ...]) -> None: with zipfile.ZipFile( destination, "w", compression=zipfile.ZIP_DEFLATED, compresslevel=6 @@ -597,14 +612,7 @@ def _write_archive(destination: Path, files: tuple[SelectedFile, ...]) -> None: raise http.CloudError(f"could not safely read {item.archive_name}: {exc}") from exc with os.fdopen(descriptor, "rb") as source_file: current = os.fstat(source_file.fileno()) - if ( - not stat.S_ISREG(current.st_mode) - or current.st_size != item.size - or current.st_dev != item.device - or current.st_ino != item.inode - or current.st_mtime_ns != item.mtime_ns - or current.st_ctime_ns != item.ctime_ns - ): + if not _is_stat_unchanged(current, item): raise http.CloudError( f"{item.archive_name} changed while the source archive was being built; " "retry." @@ -624,15 +632,7 @@ def _write_archive(destination: Path, files: tuple[SelectedFile, ...]) -> None: target.write(chunk) remaining -= len(chunk) final = os.fstat(source_file.fileno()) - if ( - source_file.read(1) - or not stat.S_ISREG(final.st_mode) - or final.st_size != item.size - or final.st_dev != item.device - or final.st_ino != item.inode - or final.st_mtime_ns != item.mtime_ns - or final.st_ctime_ns != item.ctime_ns - ): + if source_file.read(1) or not _is_stat_unchanged(final, item): raise http.CloudError( f"{item.archive_name} changed while the source archive was being " "built; retry." diff --git a/tests/test_cloud_source_upload.py b/tests/test_cloud_source_upload.py index dcd40baa..0284ebee 100644 --- a/tests/test_cloud_source_upload.py +++ b/tests/test_cloud_source_upload.py @@ -2,6 +2,7 @@ from __future__ import annotations +import dataclasses import json import os import shutil @@ -274,6 +275,64 @@ def test_source_archive_rejects_same_inode_same_size_change_after_review(tmp_pat source_upload._write_archive(tmp_path / "source.zip", manifest.files) +def test_source_archive_ignores_windows_ctime_settling_on_unchanged_files( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + source_path = tmp_path / "app.py" + source_path.write_bytes(b"safe") + manifest = source_upload.select_source(tmp_path) + selected = manifest.files[0] + + # Simulate a settled/drifted ctime_ns while mtime, size, device, and inode stay identical + drifted_selected = dataclasses.replace(selected, ctime_ns=selected.ctime_ns + 5_000_000) + + # On Windows, ctime settling must NOT reject unchanged files + monkeypatch.setattr(os, "name", "nt") + archive_path = tmp_path / "source_nt.zip" + source_upload._write_archive(archive_path, (drifted_selected,)) + assert archive_path.is_file() + + # On non-Windows platforms, ctime change represents inode change and MUST be rejected + monkeypatch.setattr(os, "name", "posix") + with pytest.raises(http.CloudError, match="changed while the source archive was being built"): + source_upload._write_archive(tmp_path / "source_posix.zip", (drifted_selected,)) + + +def test_source_archive_still_rejects_modifications_on_windows( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.setattr(os, "name", "nt") + source_path = tmp_path / "app.py" + source_path.write_bytes(b"safe") + manifest = source_upload.select_source(tmp_path) + + # 1. Size/content modified + source_path.write_bytes(b"longer content modified") + with pytest.raises(http.CloudError, match="changed while the source archive was being built"): + source_upload._write_archive(tmp_path / "source1.zip", manifest.files) + + # 2. Same size but mtime modified + source_path.write_bytes(b"evil") + selected = manifest.files[0] + os.utime( + source_path, + ns=(selected.mtime_ns + 1_000_000, selected.mtime_ns + 1_000_000), + ) + with pytest.raises(http.CloudError, match="changed while the source archive was being built"): + source_upload._write_archive(tmp_path / "source2.zip", manifest.files) + + +@pytest.mark.skipif(os.name != "nt", reason="Windows-specific filesystem settling test") +def test_source_archive_windows_batch_creation_settling(tmp_path: Path) -> None: + for idx in range(50): + (tmp_path / f"file_{idx}.py").write_text(f"content = {idx}\n", encoding="utf-8") + + manifest = source_upload.select_source(tmp_path) + archive_path = tmp_path / "batch_source.zip" + source_upload._write_archive(archive_path, manifest.files) + assert archive_path.is_file() + + def test_source_dry_run_never_calls_the_api( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: Any ) -> None: