fix(cloud): ignore Windows creation-time settling during source packaging

On Windows (NTFS/FAT), st_ctime represents file creation time rather than POSIX inode metadata change time. Due to NTFS deferred timestamp resolution and filesystem caching, st_ctime_ns can settle asynchronously after file creation without any actual content or attribute changes.

This caused _write_archive to intermittently reject unchanged source files with CloudError: '<file> changed while the source archive was being built; retry.'

Fix this by restricting the st_ctime_ns check in _write_archive to non-Windows platforms (os.name != 'nt'). File integrity and replacement detection on Windows remain fully protected via st_mode, st_size, st_dev, st_ino, st_mtime_ns, and byte-length validation.

Closes usestrix/strix#1258
This commit is contained in:
VimalN2005 2026-09-12 10:12:35 +05:30
parent 95e085eb6c
commit 509a52eb3b
2 changed files with 76 additions and 17 deletions

View file

@ -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."

View file

@ -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: