From 438832d74e99bb4f4cfb33e69acc594bdccc37fd Mon Sep 17 00:00:00 2001 From: t Date: Wed, 16 Sep 2026 17:36:44 -0700 Subject: [PATCH 1/5] feat(platform_util): anchor Windows spec restoration at a directory handle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DW-309/DW-310 made a host without `dir_fd` fail closed: any rollback that had to restore or lifecycle-normalize a bound attempt-owned spec paused on Windows with "ACTION REQUIRED — attempt-owned spec needs manual recovery", because CPython offers no `*at()` family there. The kernel underneath Win32 has the same primitive, so build it: - `win32_at.py` (new): ctypes bindings for `NtCreateFile` with a `RootDirectory` handle (`open_directory`, `open_at`, `stat_at`) and `NtSetInformationFile` `FileRenameInformationEx` / `FileDispositionInformationEx` with POSIX semantics (`replace_at`, `unlink_at`; the classic classes as the fallback where a volume refuses the Ex ones). `AT_NOFOLLOW` opens the reparse point itself and refuses a symlink or junction with ELOOP, as `O_NOFOLLOW` refuses a link; the flag vocabulary IS `os.O_*` on POSIX and free CRT bits on Windows, so one `os.O_RDONLY | AT_NOFOLLOW` serves both arms. Handles come back as CRT descriptors (`msvcrt.open_osfhandle`) so `os.fstat`/`read`/`fsync`/ `fdopen`/`samestat` work unchanged. NTSTATUS maps through `RtlNtStatusToDosError` into the OSError subclass Win32 would raise; IsADirectoryError/NotADirectoryError are pinned by hand because the Win32 mapping loses them. Importable everywhere; ENOSYS off Windows. - `platform_util`: `HANDLE_ANCHORED_WRITES = DIR_FD_ANCHORED_WRITES or win32_at.AVAILABLE`, with `open_at`/`stat_at`/`replace_at`/`unlink_at` wrappers whose POSIX arm is the bare `dir_fd` syscall (byte-identical to before) and whose Windows arm goes through `win32_at` (`replace_at` inside `_retry_on_sharing_violation`, as `atomic_replace` is). `open_dir_confined` walks on both arms; `_open_exclusive_at`, `_atomic_write_at`, `_refuse_unwritable_target_at`, `_atomic_write_confined` and `create_exclusive_confined` route through the wrappers, so the confined spec writers anchor at a handle on Windows too. The confined TEXT writer keeps `atomic_write_text`'s translating newline default on the anchored arm (`newline=None`), so the CRLF a ledger or decisions file lands on Windows is unchanged. Callers that need the rest of the POSIX family (`scandir(fd)`, `symlink` with `dir_fd`, directory fsync) stay on `DIR_FD_ANCHORED_WRITES`. - `recovery_flow`: the two refusals gate on `HANDLE_ANCHORED_WRITES` ("without handle-anchored writes"; the journaled `problem` reads "lacks handle-anchored writes"), and the target stat/open/readback go through the wrappers. Tests: - tests/test_win32_at.py (new, skip off win32): every primitive, junction refusal, EXCL, replace over an open share-delete target, the WinError 5/32 sharing violation the retry exists for, POSIX unlink while open, the directory-rename refusal while a handle is open beneath, and the anchor following a renamed directory; one ENOSYS row runs off Windows. - test_recovery_flow.py / test_engine.py: `requires_descriptor_restoration` and the "dir-fd anchoring is POSIX-only" gates flip to `HANDLE_ANCHORED_WRITES` (the seven engine rows and ~30 recovery rows now run on Windows CI); the native-Windows refusal row becomes `test_owned_spec_restore_native_windows_anchors_at_a_handle`. Rows that rename a directory out from under the open writer are POSIX-only by the OS's own rule (Windows refuses that rename with ERROR_ACCESS_DENIED — `posix_parent_swap_under_writer`, pinned from the other side in test_win32_at.py); directory redirects are planted as junctions on Windows (`_plant_directory_redirect`), fifo/socket/file-symlink entry types skip there and a `junction` parameter joins the final-entry substitution row. - test_platform_util.py: an `ANCHORED` marker runs the platform-neutral `open_dir_confined` / `*_at` / confined-callback rows on both arms (`_dir_fd` opens through `win32_at` on Windows); EBADF asserted by errno ("Invalid handle" on Windows); a new row pins the confined text writer's platform line ending (ablation: `newline=""` reddens it on Windows). - Forced-fallback rows patch `HANDLE_ANCHORED_WRITES` alongside `DIR_FD_ANCHORED_WRITES`; `_refuse_link` joins the refusal-helper inventory. Linux: full suite, pyright and trunk clean. The Windows arm was driven under Wine (relative opens, renames, link refusal, error subclasses, stat_at S_IFLNK); Wine diverges from Windows on delete-on-close, directory renames with open handles, and a handle-relative rename after the anchored directory moved, so those rows are settled by the Windows CI legs. --- docs/FEATURES.md | 4 +- src/bmad_loop/platform_util.py | 156 ++++++--- src/bmad_loop/recovery_flow.py | 35 +- src/bmad_loop/win32_at.py | 541 +++++++++++++++++++++++++++++ tests/test_artifact_publication.py | 1 + tests/test_engine.py | 14 +- tests/test_platform_util.py | 77 ++-- tests/test_portability_guard.py | 1 + tests/test_recovery_flow.py | 180 +++++++--- tests/test_win32_at.py | 425 ++++++++++++++++++++++ 10 files changed, 1293 insertions(+), 141 deletions(-) create mode 100644 src/bmad_loop/win32_at.py create mode 100644 tests/test_win32_at.py diff --git a/docs/FEATURES.md b/docs/FEATURES.md index 8b51cbf8..5d83c30a 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -193,7 +193,7 @@ Result` section. Every other spec keeps warn-and-continue, and the record says w (#705). Sweep migration and triage tasks make the same rollover automatically when an `ESCALATED` task restarts with a fresh attempt budget; mid-flight, non-escalated restarts keep their current generation because their continuing attempt counter already provides a fresh id. -- Attempt-owned sprint-spec recovery (#123, #630): a bound plain attempt whose only residue is its own lifecycle flip is normalized back to its pre-attempt lifecycle status, proven Git-clean, and retried. Every bound retry chain snapshots its first spec input byte-for-byte and retains it across both dev-verification and review-verification repair sessions; a resolved re-drive therefore retains the operator-corrected `ready-for-dev` input rather than a failed child's later body. Repair entry points validate retained authority before constructing a prompt that can reset the spec. A non-fixable retry parks the failed child first, restores that snapshot, and re-establishes the promised route after resetting sibling residue. Descriptor-capable restoration retains the staged inode across publication and verifies that exact inode before accepting it. A platform without descriptor-relative writes refuses before staging or lifecycle normalization, leaves the current attempt-owned spec bytes untouched by restoration, and pauses for manual adoption. On descriptor-capable platforms, the same snapshot restores pre-launch operator edits when a plain child puts a tracked spec back at Git baseline. Git-ignored and pre-existing-untracked bound specs use the byte snapshot as their dirtiness oracle and are force-included only in the private recovery ref before restoration; index-only force-adds and cached removals also trigger cleanup and restore baseline index ownership. That real repair reports `rollback-owned-spec-restored`, never `rollback-skipped-clean`. Missing, unreadable, deleted, retargeted, changed external, or unsafe legacy authority pauses once with spec-specific adoption instructions and clears the unusable pair so manual recovery can converge; recovery also refuses a reset whose baseline would replace the canonical path or a parent directory with a symlink, tree, file, or other unsafe shape. An initial Sprint binding fault may safely degrade to an unbound bare-key launch; an existing Stories folder+id target instead aborts unless it can be snapshotted. Once an explicit binding is durable, a later snapshot fault aborts before child launch while retaining that authority for recovery. Fresh sprint tasks with no recorded path remain bare-key dispatches; other substantive changes or sibling residue follow rollback policy; Stories remains folder+id; Sweep remains intent-bundle routing; snapshots are retired after commit; and recovery never auto-commits the human correction. +- Attempt-owned sprint-spec recovery (#123, #630): a bound plain attempt whose only residue is its own lifecycle flip is normalized back to its pre-attempt lifecycle status, proven Git-clean, and retried. Every bound retry chain snapshots its first spec input byte-for-byte and retains it across both dev-verification and review-verification repair sessions; a resolved re-drive therefore retains the operator-corrected `ready-for-dev` input rather than a failed child's later body. Repair entry points validate retained authority before constructing a prompt that can reset the spec. A non-fixable retry parks the failed child first, restores that snapshot, and re-establishes the promised route after resetting sibling residue. Restoration is anchored at a handle on the spec's parent directory on both supported hosts — the POSIX `dir_fd` family, and on Windows NT handle-relative opens (`win32_at`: `NtCreateFile` with a `RootDirectory`, `FileRenameInformationEx`/`FileDispositionInformationEx` with POSIX semantics, reparse points refused as a link) — retains the staged inode across publication and verifies that exact inode before accepting it. A host with neither arm refuses before staging or lifecycle normalization, leaves the current attempt-owned spec bytes untouched by restoration, and pauses for manual adoption. The same snapshot restores pre-launch operator edits when a plain child puts a tracked spec back at Git baseline. Git-ignored and pre-existing-untracked bound specs use the byte snapshot as their dirtiness oracle and are force-included only in the private recovery ref before restoration; index-only force-adds and cached removals also trigger cleanup and restore baseline index ownership. That real repair reports `rollback-owned-spec-restored`, never `rollback-skipped-clean`. Missing, unreadable, deleted, retargeted, changed external, or unsafe legacy authority pauses once with spec-specific adoption instructions and clears the unusable pair so manual recovery can converge; recovery also refuses a reset whose baseline would replace the canonical path or a parent directory with a symlink, tree, file, or other unsafe shape. An initial Sprint binding fault may safely degrade to an unbound bare-key launch; an existing Stories folder+id target instead aborts unless it can be snapshotted. Once an explicit binding is durable, a later snapshot fault aborts before child launch while retaining that authority for recovery. Fresh sprint tasks with no recorded path remain bare-key dispatches; other substantive changes or sibling residue follow rollback policy; Stories remains folder+id; Sweep remains intent-bundle routing; snapshots are retired after commit; and recovery never auto-commits the human correction. - Intent-gap patch-restore (BMAD-METHOD#2564): when review halts on an `intent gap`, the dev primitive saves the attempted change as a patch file (referenced from the halt output) before reverting the tree. If that reading turns out to be correct, the resolve agent adds `"restore_patch": ""` to its `resolution.json`; the orchestrator re-arms the spec to `in-review` (not `ready-for-dev`) and re-applies the patch after every reset, so the re-driven session resumes _review_ on the restored diff instead of re-implementing. `bmad-loop resolve --no-interactive --restore-patch ` does the same by hand. A patch that fails to apply escalates rather than dispatching onto a half-restored tree. Sweep bundles get the same recovery. ### Git worktree isolation (opt-in) @@ -708,7 +708,7 @@ persisted artifacts. ### Configuration (`.bmad-loop/policy.toml`) - Single policy file written by `init`, stamped into the run at every engine start — `run`, `sweep`, `resume` — so it always describes the policy that process enforces (applies to new runs and resumes; editable live from the TUI). -- Rewrites of it are confined and permission-honoring (#593, #597). On POSIX such a write walks the components below the project `O_NOFOLLOW` and lands through the descriptor that walk produced; so a symlink planted at `.bmad-loop/` is refused rather than followed — refusing a link at the file alone never covered its parent, and `mkdir(parents=True, exist_ok=True)` accepts a symlinked directory. Windows has no `*at()` family and degrades to a documented check-then-write; `init`'s one-time seeding of a missing policy predates any session and stays a plain write. A `policy.toml` an operator marked read-only is refused with a `PermissionError` instead of being replaced and left still reading `0444`. The confined walk also covers story specs inside the checkout, park records, the decisions store, and the sweep's triage cache and bundle intent document (DW-269); the read-only refusal reaches further — story specs, `sprint-status.yaml`, park records, the decisions store, hook `settings.json` — but `sprint-status.yaml` deliberately keeps its symlink-following writer (an operator may keep the board behind a link) and the hook-settings and worktree-provisioning writers keep their own pre-existing link checks rather than the descriptor walk. The read-only refusal deliberately skips machine-minted state (run archives, stop requests, the config-digest stamp) — those are channels, not operator signals. +- Rewrites of it are confined and permission-honoring (#593, #597). Such a write walks the components below the project no-follow and lands through the directory handle that walk produced — `O_DIRECTORY|O_NOFOLLOW` opens with `dir_fd` on POSIX, `NtCreateFile` relative to the handle above with reparse points refused on Windows (`win32_at`) — so a symlink or junction planted at `.bmad-loop/` is refused rather than followed; refusing a link at the file alone never covered its parent, and `mkdir(parents=True, exist_ok=True)` accepts a symlinked directory. A host with neither arm degrades to a documented check-then-write; `init`'s one-time seeding of a missing policy predates any session and stays a plain write. A `policy.toml` an operator marked read-only is refused with a `PermissionError` instead of being replaced and left still reading `0444`. The confined walk also covers story specs inside the checkout, park records, the decisions store, and the sweep's triage cache and bundle intent document (DW-269); the read-only refusal reaches further — story specs, `sprint-status.yaml`, park records, the decisions store, hook `settings.json` — but `sprint-status.yaml` deliberately keeps its symlink-following writer (an operator may keep the board behind a link) and the hook-settings and worktree-provisioning writers keep their own pre-existing link checks rather than the descriptor walk. The read-only refusal deliberately skips machine-minted state (run archives, stop requests, the config-digest stamp) — those are channels, not operator signals. - Sections — all 15: `[gates]`, `[limits]`, `[verify]`, `[notify]`, `[review]`, `[stories]` (which planning pipeline drives the loop: sprint-status or a typed `stories.yaml`), `[dev]` (see below), `[adapter]` (+ per-stage `[adapter.dev|review|triage]`), `[sweep]`, `[scm]` (worktree isolation + merge-back), `[cleanup]` (run-dir retention + disk reclamation), `[plugins]` (trust allowlist + per-plugin `[plugins.]` config — e.g. the opt-in game-engine layer via `[plugins.unity]`, off by default), `[tui]` (`low_frame_rate` for slow/SSH links; persisted dashboard pane sizes), `[operator]` (whether a dev session may park a story at `awaiting-operator`), `[mux]` (machine-scoped multiplexer backend choice). - `[dev] skill` names the inner dev skill the orchestrator drives. `"bmad-dev-auto"` — the generic upstream dev primitive — is the only accepted value; the field is retained as the seam for a future alternative dev skill, and any other value is rejected at load. It is **not** the name sessions are dispatched with: upstream renamed the primitive to `bmad-build-auto`, so the invoked name is resolved from what is actually installed and a project on either era works with this field untouched. It has no entry in the core settings schema, so it is edited in the file rather than from the TUI settings editor. - Tunable limits: `max_review_cycles`, `max_dev_attempts`, `artifact_file_max_mb`, `artifact_payload_max_mb` (binary MiB, exactly 1,048,576 raw bytes each), `max_followup_reviews`, `session_timeout_min`, `git_timeout_s`, `teardown_grace_s` (one shared budget bounding the verified window kill _and_ the follow-on reap of any straggler descendant the session detached — e.g. a `setsid` background writer — combined; whatever remains after the window dies is what the straggler reap gets, before the worktree is merged and removed), `stop_without_result_nudges`, `dev_stall_grace_s`, `dev_stall_nudges`, `dev_stall_nudges_cap`, `workflow_stall_nudges_cap`, `max_tokens_per_story`. diff --git a/src/bmad_loop/platform_util.py b/src/bmad_loop/platform_util.py index e9e3de67..c3d77ec7 100644 --- a/src/bmad_loop/platform_util.py +++ b/src/bmad_loop/platform_util.py @@ -34,6 +34,7 @@ from pathlib import Path, PurePosixPath, PureWindowsPath from typing import Callable, Iterator +from . import win32_at from .process_host import get_process_host # Windows-only: os.replace (MoveFileExW) fails with ERROR_ACCESS_DENIED (5) or @@ -854,6 +855,57 @@ def _atomic_write( # Windows, whose pyconfig has neither HAVE_RENAMEAT nor HAVE_OPENAT. DIR_FD_ANCHORED_WRITES = hasattr(os, "O_DIRECTORY") +# Whether a write can be anchored to an open directory HANDLE at all — POSIX +# through the ``*at()`` family above, Windows through the NT handle-relative +# opens in :mod:`.win32_at`. The anchored helpers below (`open_dir_confined`, +# the ``*_at`` writers, the confined writers) run on either arm behind this +# flag; :data:`DIR_FD_ANCHORED_WRITES` stays the narrower question for callers +# that need the rest of the POSIX family too (``scandir(fd)``, ``symlink`` with +# ``dir_fd``, ``fsync`` of a directory), which has no Windows spelling here. +HANDLE_ANCHORED_WRITES = DIR_FD_ANCHORED_WRITES or win32_at.AVAILABLE + +# The flag vocabulary of :func:`open_at`, spelled once for both arms: the real +# ``os.O_*`` bits on POSIX, and bits the CRT's set leaves free on Windows. +AT_NOFOLLOW = win32_at.AT_NOFOLLOW +AT_NONBLOCK = win32_at.AT_NONBLOCK +AT_DIRECTORY = win32_at.AT_DIRECTORY + + +def open_at(dir_fd: int, name: str, flags: int, mode: int = 0o600) -> int: + """``os.open(name, flags, mode, dir_fd=dir_fd)`` on whichever arm the host + has. ``name`` is one component; ``flags`` may carry :data:`AT_NOFOLLOW`, + :data:`AT_NONBLOCK` and :data:`AT_DIRECTORY` alongside the access bits.""" + if DIR_FD_ANCHORED_WRITES: + return os.open(name, flags, mode, dir_fd=dir_fd) + return win32_at.open_at(dir_fd, name, flags, mode) + + +def stat_at(dir_fd: int, name: str) -> os.stat_result: + """``os.stat(name, dir_fd=dir_fd, follow_symlinks=False)`` on either arm: the + entry's own metadata, a link reported as ``S_IFLNK`` rather than followed.""" + if DIR_FD_ANCHORED_WRITES: + return os.stat(name, dir_fd=dir_fd, follow_symlinks=False) + return win32_at.stat_at(dir_fd, name) + + +def replace_at(src_dir_fd: int, src: str, dst_dir_fd: int, dst: str) -> None: + """``os.replace(src, dst, src_dir_fd=..., dst_dir_fd=...)`` on either arm. + The Windows arm retries the transient sharing violation a concurrent handle + on ``dst`` raises, as :func:`atomic_replace` does for the path-based writer; + POSIX rename-over-open never raises it, so that arm is the bare syscall.""" + if DIR_FD_ANCHORED_WRITES: + os.replace(src, dst, src_dir_fd=src_dir_fd, dst_dir_fd=dst_dir_fd) + return + _retry_on_sharing_violation(lambda: win32_at.replace_at(src_dir_fd, src, dst_dir_fd, dst)) + + +def unlink_at(dir_fd: int, name: str) -> None: + """``os.unlink(name, dir_fd=dir_fd)`` on either arm.""" + if DIR_FD_ANCHORED_WRITES: + os.unlink(name, dir_fd=dir_fd) + return + win32_at.unlink_at(dir_fd, name) + # Windows reparse tags that make a directory entry REDIRECT somewhere else, # compared against os.lstat().st_reparse_tag (Windows, 3.8+). Deliberately not @@ -881,11 +933,13 @@ def is_link_like(path: Path) -> bool: a directory symlink needs SeCreateSymbolicLinkPrivilege or Developer Mode, so the UNPRIVILEGED redirect is exactly the one an ``is_symlink()`` check misses. - This is the win32 half of :func:`open_dir_confined`, which anchors the POSIX - side at a descriptor instead. A path check is inherently check-then-write — - answered about a name, and stale the moment it returns — so it narrows the - window rather than closing it. That residual is the platform's, not this - function's: win32 has no ``*at()`` family to anchor against. + This is the path-based half of :func:`open_dir_confined`, for the callers + that still walk by name (the ones needing the rest of the POSIX ``dir_fd`` + family, which :data:`DIR_FD_ANCHORED_WRITES` gates). A path check is + inherently check-then-write — answered about a name, and stale the moment it + returns — so it narrows the window rather than closing it; the confined + writers themselves anchor at a handle on both arms since + :data:`HANDLE_ANCHORED_WRITES`. ``events.py`` and the standalone hook relay keep their own copies of this predicate on purpose: they run under the HOST's interpreter, not this @@ -920,12 +974,12 @@ def path_is_confined(root: Path, target: Path) -> bool: """Whether ``target`` is reached from ``root`` without traversing a redirect at any component below it. - The win32 half of :func:`open_dir_confined`, which anchors the POSIX side at - a descriptor instead. A check, not a race-free open: it is answered about a - NAME and is stale the moment it returns, so it removes the standing redirect - — plant a link, wait for a write — while a writer who re-plants inside the - window between check and write still wins. That residual is the platform's, - not this function's: win32 has no ``*at()`` family to anchor against. + The path-based half of :func:`open_dir_confined`, for callers that must + walk by name. A check, not a race-free open: it is answered about a NAME and + is stale the moment it returns, so it removes the standing redirect — plant + a link, wait for a write — while a writer who re-plants inside the window + between check and write still wins. Callers that can anchor at a handle + (:data:`HANDLE_ANCHORED_WRITES`, both arms) do not carry that residual. Every component below ``root`` is checked and ``root`` itself is not: the operator chooses where the project lives and may well keep it behind a link, @@ -1031,9 +1085,13 @@ def open_dir_confined(root: Path, target: Path, *, search_only: bool = False) -> ancestors without weakening the readable default; hosts with neither flag retain the readable behavior. - POSIX only — see :data:`DIR_FD_ANCHORED_WRITES`. Callers need a fallback for - win32, which has no ``*at()`` family to anchor against.""" - if not DIR_FD_ANCHORED_WRITES: + Both arms of :data:`HANDLE_ANCHORED_WRITES`: the ``*at()`` walk on POSIX, + and on Windows the same walk through :mod:`.win32_at`'s handle-relative + opens — a directory handle for ``root``, then each component opened relative + to the one above with reparse points refused, so a junction or symlink below + the root fails the walk exactly as ``O_NOFOLLOW`` fails it. Hosts with + neither arm get None, and their callers keep a path-based fallback.""" + if not HANDLE_ANCHORED_WRITES: return None try: relative = target.relative_to(root) @@ -1042,12 +1100,15 @@ def open_dir_confined(root: Path, target: Path, *, search_only: bool = False) -> search_access = getattr(os, "O_SEARCH", 0) or getattr(os, "O_PATH", 0) access = search_access if search_only else os.O_RDONLY try: - fd = os.open(root, access | os.O_DIRECTORY) + if DIR_FD_ANCHORED_WRITES: + fd = os.open(root, access | os.O_DIRECTORY) + else: + fd = win32_at.open_directory(root) except OSError: return None for part in relative.parts: try: - nested = os.open(part, access | os.O_DIRECTORY | os.O_NOFOLLOW, dir_fd=fd) + nested = open_at(fd, part, access | AT_DIRECTORY | AT_NOFOLLOW) except OSError: os.close(fd) return None # a link, a missing component, or one we cannot probe @@ -1078,9 +1139,10 @@ def atomic_write_text_at(dir_fd: int, name: str, text: str) -> None: keeps the private ``0600`` it is created with. Text is written UTF-8 with no newline translation; the callers are records, not operator-edited files. - No win32 sharing-violation retry, unlike :func:`atomic_replace`: there is no - win32 here at all — the ``*at()`` family this is built on does not exist - there, so a caller reaching this is on POSIX by construction.""" + Both arms of :data:`HANDLE_ANCHORED_WRITES`: on POSIX every step is the + ``dir_fd`` syscall named above; on Windows the same step through + :mod:`.win32_at`'s handle-relative opens, where :func:`replace_at` also + carries the sharing-violation retry :func:`atomic_replace` makes.""" _atomic_write_at(dir_fd, name, text, mode="w", encoding="utf-8") @@ -1097,7 +1159,7 @@ def atomic_write_bytes_at( """:func:`atomic_write_text_at`'s byte-exact sibling, whose docstring carries the shared contract (a unique unguessable temp created ``O_EXCL`` at ``0600``, every syscall relative to ``dir_fd``, fsync before the replace, temp removed - on any failure, no mode or xattrs inherited, POSIX by construction). + on any failure, no mode or xattrs inherited, both anchored arms). The one difference is the whole point: ``data`` lands byte-for-byte. No encode and no newline translation, so a payload carrying CRLF keeps CRLF and @@ -1141,7 +1203,7 @@ def _open_exclusive_at(dir_fd: int, prefix: str, name: str) -> tuple[int, str]: for _ in range(_TMP_NAME_ATTEMPTS): tmp = f"{prefix}{os.getpid():x}.{os.urandom(4).hex()}.tmp" try: - return os.open(tmp, os.O_RDWR | os.O_CREAT | os.O_EXCL, 0o600, dir_fd=dir_fd), tmp + return open_at(dir_fd, tmp, os.O_RDWR | os.O_CREAT | os.O_EXCL, 0o600), tmp except FileExistsError: continue # astronomically unlikely; costs one more draw raise OSError(f"no free temp name beside {name!r} after {_TMP_NAME_ATTEMPTS} tries") @@ -1158,13 +1220,17 @@ def _atomic_write_at( before_staging: Callable[[], None] | None = None, before_replace: Callable[[], None] | None = None, after_replace: Callable[[int], None] | None = None, + newline: str | None = "", ) -> None: """The shared body of the two anchored helpers above — see :func:`atomic_write_text_at` for the contract every step here implements. ``encoding`` doubles as the text/bytes discriminator, as it does in - :func:`_atomic_write`: the text arm is opened with it plus ``newline=""``, - the bytes arm with neither, because ``os.fdopen`` refuses both in binary mode + :func:`_atomic_write`: the text arm is opened with it plus ``newline`` — + ``""`` (no translation) for the record writers above, the translating + ``None`` when the confined text writer routes here so the bytes it lands + on Windows stay the CRLF the path-based writer has always landed — the + bytes arm with neither, because ``os.fdopen`` refuses both in binary mode and a byte-verbatim payload has nothing to translate anyway. Staging walks :func:`_stage_shortening`'s ladder, the same one @@ -1182,7 +1248,7 @@ def _atomic_write_at( staged = ( os.fdopen(fd, mode) if encoding is None - else os.fdopen(fd, mode, encoding=encoding, newline="") + else os.fdopen(fd, mode, encoding=encoding, newline=newline) ) with staged as fh: fh.write(payload) @@ -1190,14 +1256,14 @@ def _atomic_write_at( os.fsync(fh.fileno()) if before_replace is not None: before_replace() - os.replace(tmp, name, src_dir_fd=dir_fd, dst_dir_fd=dir_fd) + replace_at(dir_fd, tmp, dir_fd, name) published = True if after_replace is not None: after_replace(fh.fileno()) except BaseException: if not published: with suppress(OSError): - os.unlink(tmp, dir_fd=dir_fd) + unlink_at(dir_fd, tmp) raise @@ -1211,9 +1277,9 @@ def _refuse_unwritable_target_at(dir_fd: int, name: str) -> None: NAME — the same reasoning as the no-follow arm there, ``ELOOP`` included. ``O_NONBLOCK`` for the reason the path-based probe gives: a reader-less FIFO planted at the name answers ``ENXIO`` instead of wedging the probe forever. - POSIX by construction: only :data:`DIR_FD_ANCHORED_WRITES` reaches here.""" + Either arm of :data:`HANDLE_ANCHORED_WRITES` reaches here.""" try: - fd = os.open(name, os.O_WRONLY | os.O_NOFOLLOW | os.O_NONBLOCK, dir_fd=dir_fd) + fd = open_at(dir_fd, name, os.O_WRONLY | AT_NOFOLLOW | AT_NONBLOCK) except PermissionError: raise # the refusal this flag exists for except OSError: @@ -1251,9 +1317,11 @@ def atomic_write_text_confined( already EXIST: a confinement walk cannot vouch for a component that is not there, so every adopter mkdirs or gates first. - POSIX walks the components with :func:`open_dir_confined` and writes through - the descriptor that walk produced, which a later swap of any name along the - way no longer reaches. Win32 has no ``*at()`` family, so it degrades to + Both anchored hosts walk the components with :func:`open_dir_confined` — + ``dir_fd`` opens on POSIX, :mod:`.win32_at`'s handle-relative opens on + Windows — and write through the directory handle that walk produced, which + a later swap of any name along the way no longer reaches. A host with + neither arm (:data:`HANDLE_ANCHORED_WRITES` False) degrades to :func:`path_is_confined` plus a no-follow write — check-then-write, which removes the standing redirect but leaves the window between the check and the write open (the precedent, and the same documented residual, as @@ -1262,12 +1330,12 @@ def atomic_write_text_confined( Mode and xattrs are NEVER inherited — the file lands at ``0600``, which is exactly what ``follow_symlinks=False`` already gives this cohort, so adopting - this changes no file's permissions. The anchored arm writes UTF-8 with no - newline translation (identity on POSIX, where the translating default writes - ``\n`` unchanged) and the win32 arm keeps :func:`atomic_write_text`'s - translating default, so on each platform the bytes that land are the ones - that land today. A caller preserving a file's existing line endings wants - :func:`atomic_write_bytes_confined`, as it wants the bytes writer today. + this changes no file's permissions. Every arm writes UTF-8 with + :func:`atomic_write_text`'s translating newline default — identity on POSIX, + CRLF on Windows whether the write is anchored through :mod:`.win32_at` or + falls back to the path — so on each platform the bytes that land are the + ones that land today. A caller preserving a file's existing line endings + wants :func:`atomic_write_bytes_confined`, as it wants the bytes writer today. ``require_writable_target`` behaves as it does in :func:`atomic_write_text`; on the anchored arm the probe is asked dir_fd-relative, never by path.""" @@ -1352,7 +1420,7 @@ def _atomic_write_confined( if has_parent_ref(path.relative_to(confine_root)): raise UnconfinedWriteError(f"{path} climbs back out of {confine_root} through '..'") unconfined = f"cannot reach {path.parent} from {confine_root} without a redirect" - if DIR_FD_ANCHORED_WRITES: + if HANDLE_ANCHORED_WRITES: dir_fd = open_dir_confined(confine_root, path.parent) if dir_fd is None: raise UnconfinedWriteError(unconfined) @@ -1367,6 +1435,7 @@ def _atomic_write_confined( before_staging=before_staging, before_replace=before_replace, after_replace=after_replace, + newline=None, # the path writer's translating default, both arms ) finally: os.close(dir_fd) @@ -1399,9 +1468,10 @@ def create_exclusive_confined(path: Path, *, confine_root: Path) -> int: where "is one pending?" and "lodge mine" must stay a single atomic step against the destination name. The temp-and-replace confined writers cannot express that — a replace is unconditional by design — so this shares only - their parent walk, not their staging. On POSIX the create is anchored at the - walked descriptor; win32 has no ``*at()`` family and degrades to the same - documented check-then-create as :func:`atomic_write_text_confined`'s + their parent walk, not their staging. Where a write can be anchored + (:data:`HANDLE_ANCHORED_WRITES`, POSIX and Windows alike) the create is made + relative to the walked descriptor; a host with neither arm degrades to the + same documented check-then-create as :func:`atomic_write_text_confined`'s fallback arm.""" if not path.is_relative_to(confine_root): raise UnconfinedWriteError(f"{path} is not under {confine_root}") @@ -1409,12 +1479,12 @@ def create_exclusive_confined(path: Path, *, confine_root: Path) -> int: raise UnconfinedWriteError(f"{path} climbs back out of {confine_root} through '..'") unconfined = f"cannot reach {path.parent} from {confine_root} without a redirect" flags = os.O_WRONLY | os.O_CREAT | os.O_EXCL - if DIR_FD_ANCHORED_WRITES: + if HANDLE_ANCHORED_WRITES: dir_fd = open_dir_confined(confine_root, path.parent) if dir_fd is None: raise UnconfinedWriteError(unconfined) try: - return os.open(path.name, flags, 0o600, dir_fd=dir_fd) + return open_at(dir_fd, path.name, flags, 0o600) finally: os.close(dir_fd) if not path_is_confined(confine_root, path.parent): diff --git a/src/bmad_loop/recovery_flow.py b/src/bmad_loop/recovery_flow.py index 0d5d0a0d..2d0b684c 100644 --- a/src/bmad_loop/recovery_flow.py +++ b/src/bmad_loop/recovery_flow.py @@ -24,10 +24,14 @@ from . import gates, verify from .model import Phase from .platform_util import ( - DIR_FD_ANCHORED_WRITES, + AT_NOFOLLOW, + AT_NONBLOCK, + HANDLE_ANCHORED_WRITES, atomic_write_bytes_at, + open_at, open_dir_confined, safe_ref_segment, + stat_at, ) from .statemachine import advance @@ -224,14 +228,17 @@ def _normalize_attempt_owned_spec( confinement exists for. It reaches the spec-writer chokepoint rule stated in `frontmatter.set_frontmatter_status` — an artifacts folder configured outside the project is a trusted repair target here - (`_attempt_owned_spec`) when descriptor-relative writes are available.""" + (`_attempt_owned_spec`) when handle-anchored writes are available.""" # A path-based fallback cannot retain publication authority across the # final replace. Refuse before any repair write so a substituted parent - # or target cannot redirect staging, publication, or cleanup. - if not DIR_FD_ANCHORED_WRITES: + # or target cannot redirect staging, publication, or cleanup. Both the + # POSIX `dir_fd` arm and the Windows handle-relative arm anchor + # (`platform_util.HANDLE_ANCHORED_WRITES`); only a host with neither + # reaches this refusal. + if not HANDLE_ANCHORED_WRITES: raise _OwnedSpecAuthorityError( "safe automatic attempt-owned spec restoration is unavailable because " - "it cannot be verified without descriptor-relative writes: " + "it cannot be verified without handle-anchored writes: " f"{spec_path}", safe_restoration_unavailable=True, ) @@ -291,10 +298,10 @@ def _restore_attempt_owned_spec_bytes(spec_path: Path, snapshot: bytes) -> None: f"attempt-owned spec target could not be revalidated: {spec_path}" ) from exc - if not DIR_FD_ANCHORED_WRITES: + if not HANDLE_ANCHORED_WRITES: raise _OwnedSpecAuthorityError( "safe automatic attempt-owned spec restoration is unavailable because " - "it cannot be verified without descriptor-relative writes: " + "it cannot be verified without handle-anchored writes: " f"{spec_path}", safe_restoration_unavailable=True, ) @@ -341,7 +348,7 @@ def verify_parent_authority(parent_fd: int) -> None: def target_stat_at(parent_fd: int) -> os.stat_result | None: try: - observed = os.stat(spec_path.name, dir_fd=parent_fd, follow_symlinks=False) + observed = stat_at(parent_fd, spec_path.name) except FileNotFoundError: return None if not stat.S_ISREG(observed.st_mode): @@ -354,9 +361,9 @@ def read_target_at( observed = target_stat_at(parent_fd) if observed is None: return None - flags = os.O_RDONLY | os.O_NOFOLLOW | os.O_NONBLOCK + flags = os.O_RDONLY | AT_NOFOLLOW | AT_NONBLOCK try: - target_fd = os.open(spec_path.name, flags, dir_fd=parent_fd) + target_fd = open_at(parent_fd, spec_path.name, flags) except OSError as exc: if exc.errno in { errno.ELOOP, @@ -425,9 +432,9 @@ def verify_published_inode(parent_fd: int, published_fd: int) -> None: # The writer keeps this exact staged inode open across publication. # Opening the live name no-follow/nonblocking proves it still names # that inode without following a link or waiting on a planted FIFO. - flags = os.O_RDONLY | os.O_NOFOLLOW | os.O_NONBLOCK + flags = os.O_RDONLY | AT_NOFOLLOW | AT_NONBLOCK try: - live_fd = os.open(spec_path.name, flags, dir_fd=parent_fd) + live_fd = open_at(parent_fd, spec_path.name, flags) except OSError as exc: if exc.errno in { errno.ELOOP, @@ -465,7 +472,7 @@ def verify_published_inode(parent_fd: int, published_fd: int) -> None: raise _OwnedSpecAuthorityError(authority_message) try: - named = os.stat(spec_path.name, dir_fd=parent_fd, follow_symlinks=False) + named = stat_at(parent_fd, spec_path.name) except OSError as exc: if exc.errno in {errno.ELOOP, errno.ENOENT, errno.ENOTDIR}: raise _OwnedSpecAuthorityError(authority_message) from exc @@ -545,7 +552,7 @@ def _owned_spec_restore_problem( ) return ( f"safe automatic restoration is unavailable {unsafe_context} because " - "this platform lacks descriptor-relative writes" + "this platform lacks handle-anchored writes" f"{status_guidance}; manual adoption is required" ) return f"its path became unsafe {unsafe_context} ({exc})" diff --git a/src/bmad_loop/win32_at.py b/src/bmad_loop/win32_at.py new file mode 100644 index 00000000..00eb7ecc --- /dev/null +++ b/src/bmad_loop/win32_at.py @@ -0,0 +1,541 @@ +"""Handle-relative filesystem primitives for Windows — the ``*at()`` family +the anchored writers are built on, where CPython offers none. + +POSIX anchors a write to an open directory descriptor: ``openat``/``renameat`` +/``unlinkat`` name a single component *relative to that descriptor*, so a +concurrent rename of any directory along the original path renames something +the writer no longer consults. CPython exposes that family as ``dir_fd=``, and +implements it with the ``*at`` calls only — ``os.supports_dir_fd`` is empty on +Windows. The kernel underneath Win32 has the same primitive, though: every NT +open takes an ``OBJECT_ATTRIBUTES`` whose ``RootDirectory`` is a handle the +name is resolved against, and a rename or delete is a ``NtSetInformationFile`` +on the file's own handle, with the destination again spelled relative to a +handle. This module is the thin ``ctypes`` binding of exactly those calls: + +- :func:`open_directory` — a directory handle, wrapped into a CRT descriptor; +- :func:`open_at` — ``NtCreateFile`` relative to that descriptor, with the + ``O_NOFOLLOW`` / ``O_DIRECTORY`` / ``O_CREAT|O_EXCL`` flag vocabulary the + POSIX callers already speak; +- :func:`stat_at` — an ``lstat`` of one entry, taken through a handle; +- :func:`replace_at` / :func:`unlink_at` — ``FileRenameInformation`` / + ``FileDispositionInformation`` relative to a directory handle. + +Every handle comes back as a CRT file descriptor (``msvcrt.open_osfhandle``), +so the callers keep using ``os.fstat``, ``os.read``, ``os.fsync``, +``os.fdopen`` and ``os.close`` on it unchanged — including +``os.path.samestat``, whose ``st_ino``/``st_dev`` CPython fills from the +handle's file index and volume serial. The one thing no CRT call gives back +is the handle-relative open itself, which is why this module exists. + +"Do not follow" is ``FILE_OPEN_REPARSE_POINT`` plus an attribute check on the +opened handle: the open lands on the reparse point itself, and a symlink or +mount point there is refused with ``ELOOP`` exactly as ``O_NOFOLLOW`` refuses +it. Only those two tags count as links — cloud placeholders and dedup stubs +are reparse points too, and refusing them would stall a legitimate run +(``platform_util._LINK_REPARSE_TAGS`` draws the same line). + +Errors are ordinary ``OSError``s: the NTSTATUS is mapped through +``RtlNtStatusToDosError`` and handed to ``OSError`` as ``winerror``, which +CPython turns into the errno and subclass Win32 would have produced +(``FileNotFoundError``, ``FileExistsError``, ``PermissionError`` for a sharing +violation, ...). Two are pinned by hand because the Win32 mapping loses them: +``STATUS_FILE_IS_A_DIRECTORY`` is ``IsADirectoryError`` and +``STATUS_NOT_A_DIRECTORY`` is ``NotADirectoryError``, the answers the POSIX +arm's callers branch on. + +Importable everywhere; every function raises ``OSError(ENOSYS)`` off +Windows, and :data:`AVAILABLE` says which arm the host is on. The bindings +themselves are only built under ``sys.platform == "win32"``. +""" + +from __future__ import annotations + +import errno +import os +import stat +import struct +import sys +from pathlib import Path + +AVAILABLE = sys.platform == "win32" + +# The flag vocabulary `open_at` understands, spelled for the POSIX callers. +# On POSIX these ARE `os.O_NOFOLLOW` etc.; here they are bits the CRT's own +# `O_*` set leaves free (it uses the low 16), so an `os.O_RDWR | AT_NOFOLLOW` +# composed by a caller means the same thing on both arms. +AT_NOFOLLOW = getattr(os, "O_NOFOLLOW", 0x0010_0000) +AT_NONBLOCK = getattr(os, "O_NONBLOCK", 0x0020_0000) +AT_DIRECTORY = getattr(os, "O_DIRECTORY", 0x0040_0000) + +_IO_REPARSE_TAG_MOUNT_POINT = 0xA0000003 +_IO_REPARSE_TAG_SYMLINK = 0xA000000C +_LINK_TAGS = frozenset({_IO_REPARSE_TAG_MOUNT_POINT, _IO_REPARSE_TAG_SYMLINK}) +_FILE_ATTRIBUTE_DIRECTORY = 0x10 +_FILE_ATTRIBUTE_REPARSE_POINT = 0x400 + + +def _unsupported() -> OSError: + return OSError(errno.ENOSYS, "handle-relative file operations are Windows-only") + + +if sys.platform == "win32": + import ctypes + import ctypes.wintypes as wt + import msvcrt + + _kernel32 = ctypes.WinDLL("kernel32", use_last_error=True) + _ntdll = ctypes.WinDLL("ntdll") + + # --- access rights / options (winnt.h, ntifs.h) --------------------------- + _DELETE = 0x0001_0000 + _SYNCHRONIZE = 0x0010_0000 + _FILE_READ_DATA = 0x0001 # == FILE_LIST_DIRECTORY on a directory + _FILE_WRITE_DATA = 0x0002 + _FILE_APPEND_DATA = 0x0004 + _FILE_READ_EA = 0x0008 + _FILE_WRITE_EA = 0x0010 + _FILE_TRAVERSE = 0x0020 + _FILE_READ_ATTRIBUTES = 0x0080 + _FILE_WRITE_ATTRIBUTES = 0x0100 + _READ_CONTROL = 0x0002_0000 + _FILE_GENERIC_READ = ( + _READ_CONTROL | _FILE_READ_DATA | _FILE_READ_ATTRIBUTES | _FILE_READ_EA | _SYNCHRONIZE + ) + _FILE_GENERIC_WRITE = ( + _READ_CONTROL + | _FILE_WRITE_DATA + | _FILE_WRITE_ATTRIBUTES + | _FILE_WRITE_EA + | _FILE_APPEND_DATA + | _SYNCHRONIZE + ) + _FILE_SHARE_ALL = 0x1 | 0x2 | 0x4 # READ | WRITE | DELETE + _FILE_ATTRIBUTE_NORMAL = 0x80 + _FILE_OPEN = 1 + _FILE_CREATE = 2 + _FILE_OPEN_IF = 3 + _FILE_DIRECTORY_FILE = 0x0001 + _FILE_NON_DIRECTORY_FILE = 0x0040 + _FILE_SYNCHRONOUS_IO_NONALERT = 0x0020 + _FILE_OPEN_REPARSE_POINT = 0x0020_0000 + _OBJ_CASE_INSENSITIVE = 0x40 + _OPEN_EXISTING = 3 + _FILE_FLAG_BACKUP_SEMANTICS = 0x0200_0000 + _FILE_FLAG_OPEN_REPARSE_POINT = 0x0020_0000 + _INVALID_HANDLE_VALUE = ctypes.c_void_p(-1).value + + # NTSTATUS values with a dedicated translation. + _STATUS_FILE_IS_A_DIRECTORY = 0xC00000BA + _STATUS_NOT_A_DIRECTORY = 0xC0000103 + # "This information class is not one this kernel/volume takes": the `Ex` + # rename/dispose classes are Windows 10 1709+ and NTFS/ReFS only. + _STATUS_INVALID_INFO_CLASS = 0xC0000003 + _STATUS_INVALID_PARAMETER = 0xC000000D + _STATUS_NOT_SUPPORTED = 0xC00000BB + _EX_CLASS_UNSUPPORTED = frozenset( + {_STATUS_INVALID_INFO_CLASS, _STATUS_INVALID_PARAMETER, _STATUS_NOT_SUPPORTED} + ) + + # FILE_INFORMATION_CLASS members (ntifs.h) and the FILE_INFO_BY_HANDLE_CLASS + # member GetFileInformationByHandleEx takes. + _FileRenameInformation = 10 + _FileDispositionInformation = 13 + _FileDispositionInformationEx = 64 + _FileRenameInformationEx = 65 + _FileAttributeTagInfo = 9 + _FILE_RENAME_REPLACE_IF_EXISTS = 0x1 + _FILE_RENAME_POSIX_SEMANTICS = 0x2 + _FILE_DISPOSITION_DELETE = 0x1 + _FILE_DISPOSITION_POSIX_SEMANTICS = 0x2 + + class _UNICODE_STRING(ctypes.Structure): + _fields_ = ( + ("Length", wt.USHORT), + ("MaximumLength", wt.USHORT), + ("Buffer", wt.LPWSTR), + ) + + class _OBJECT_ATTRIBUTES(ctypes.Structure): + _fields_ = ( + ("Length", wt.ULONG), + ("RootDirectory", wt.HANDLE), + ("ObjectName", ctypes.POINTER(_UNICODE_STRING)), + ("Attributes", wt.ULONG), + ("SecurityDescriptor", ctypes.c_void_p), + ("SecurityQualityOfService", ctypes.c_void_p), + ) + + class _IO_STATUS_BLOCK(ctypes.Structure): + _fields_ = (("Status", ctypes.c_void_p), ("Information", ctypes.c_void_p)) + + class _FILE_ATTRIBUTE_TAG_INFO(ctypes.Structure): + _fields_ = (("FileAttributes", wt.DWORD), ("ReparseTag", wt.DWORD)) + + _ntdll.NtCreateFile.restype = ctypes.c_long + _ntdll.NtCreateFile.argtypes = ( + ctypes.POINTER(wt.HANDLE), + wt.DWORD, + ctypes.POINTER(_OBJECT_ATTRIBUTES), + ctypes.POINTER(_IO_STATUS_BLOCK), + ctypes.c_void_p, + wt.ULONG, + wt.ULONG, + wt.ULONG, + wt.ULONG, + ctypes.c_void_p, + wt.ULONG, + ) + _ntdll.NtSetInformationFile.restype = ctypes.c_long + _ntdll.NtSetInformationFile.argtypes = ( + wt.HANDLE, + ctypes.POINTER(_IO_STATUS_BLOCK), + ctypes.c_void_p, + wt.ULONG, + ctypes.c_int, + ) + _ntdll.RtlNtStatusToDosError.restype = wt.ULONG + _ntdll.RtlNtStatusToDosError.argtypes = (wt.ULONG,) + _kernel32.CreateFileW.restype = wt.HANDLE + _kernel32.CreateFileW.argtypes = ( + wt.LPCWSTR, + wt.DWORD, + wt.DWORD, + ctypes.c_void_p, + wt.DWORD, + wt.DWORD, + wt.HANDLE, + ) + _kernel32.GetFileInformationByHandleEx.restype = wt.BOOL + _kernel32.GetFileInformationByHandleEx.argtypes = ( + wt.HANDLE, + ctypes.c_int, + ctypes.c_void_p, + wt.DWORD, + ) + _kernel32.CloseHandle.restype = wt.BOOL + _kernel32.CloseHandle.argtypes = (wt.HANDLE,) + + def _nt_error(status: int, name: str) -> OSError: + """The ``OSError`` Win32 would have raised for this NTSTATUS.""" + status &= 0xFFFFFFFF + if status == _STATUS_FILE_IS_A_DIRECTORY: + return IsADirectoryError(errno.EISDIR, "Is a directory", name) + if status == _STATUS_NOT_A_DIRECTORY: + return NotADirectoryError(errno.ENOTDIR, "Not a directory", name) + winerror = int(_ntdll.RtlNtStatusToDosError(status)) + # `winerror` decides the errno AND the subclass; the 0 is discarded. + return OSError(0, ctypes.FormatError(winerror).strip(), name, winerror) + + def _win_error(name: str) -> OSError: + winerror = ctypes.get_last_error() + return OSError(0, ctypes.FormatError(winerror).strip(), name, winerror) + + def _handle_of(fd: int) -> int: + return msvcrt.get_osfhandle(fd) + + def _wrap(handle: int, *, writable: bool) -> int: + """Hand a raw handle to the CRT; from here on ``os.close`` owns it.""" + flags = os.O_BINARY | os.O_NOINHERIT | (os.O_RDWR if writable else os.O_RDONLY) + try: + return msvcrt.open_osfhandle(handle, flags) + except OSError: + _kernel32.CloseHandle(handle) + raise + + def _single_component(name: str) -> None: + if not name or name in (".", "..") or "\\" in name or "/" in name or "\x00" in name: + raise ValueError(f"not a single path component: {name!r}") + + def _attribute_tag(handle: int, name: str) -> tuple[int, int]: + info = _FILE_ATTRIBUTE_TAG_INFO() + if not _kernel32.GetFileInformationByHandleEx( + handle, _FileAttributeTagInfo, ctypes.byref(info), ctypes.sizeof(info) + ): + raise _win_error(name) + return int(info.FileAttributes), int(info.ReparseTag) + + def _is_link(attributes: int, tag: int) -> bool: + return bool(attributes & _FILE_ATTRIBUTE_REPARSE_POINT) and tag in _LINK_TAGS + + def _nt_open( + root_handle: int | None, + name: str, + *, + access: int, + disposition: int, + options: int, + attributes: int = _FILE_ATTRIBUTE_NORMAL, + ) -> int: + buf = ctypes.create_unicode_buffer(name) + length = len(name) * 2 + unicode = _UNICODE_STRING(length, length + 2, ctypes.cast(buf, wt.LPWSTR)) + attrs = _OBJECT_ATTRIBUTES( + ctypes.sizeof(_OBJECT_ATTRIBUTES), + root_handle, + ctypes.pointer(unicode), + _OBJ_CASE_INSENSITIVE, + None, + None, + ) + iosb = _IO_STATUS_BLOCK() + handle = wt.HANDLE() + status = _ntdll.NtCreateFile( + ctypes.byref(handle), + access | _SYNCHRONIZE, + ctypes.byref(attrs), + ctypes.byref(iosb), + None, + attributes, + _FILE_SHARE_ALL, + disposition, + options | _FILE_SYNCHRONOUS_IO_NONALERT, + None, + 0, + ) + if status < 0: + raise _nt_error(status, name) + assert handle.value is not None + return handle.value + + def _refuse_link(handle: int, name: str) -> None: + """Close ``handle`` and raise ``ELOOP`` if it is a symlink or mount point — + the answer ``O_NOFOLLOW`` gives for the same entry.""" + try: + attributes, tag = _attribute_tag(handle, name) + except OSError: + _kernel32.CloseHandle(handle) + raise + if _is_link(attributes, tag): + _kernel32.CloseHandle(handle) + raise OSError(errno.ELOOP, "Too many levels of symbolic links", name) + + def open_directory(path: Path, *, follow: bool = True) -> int: + """A descriptor on the directory at ``path`` — the anchor every other + call here is relative to. ``follow=False`` refuses a symlink or junction + AT ``path`` with ``ELOOP``; the default follows it, as the POSIX walk's + root open does (the operator chose where the project lives).""" + flags = _FILE_FLAG_BACKUP_SEMANTICS | (0 if follow else _FILE_FLAG_OPEN_REPARSE_POINT) + handle = _kernel32.CreateFileW( + str(path), + _FILE_READ_DATA | _FILE_READ_ATTRIBUTES | _FILE_TRAVERSE, + _FILE_SHARE_ALL, + None, + _OPEN_EXISTING, + flags, + None, + ) + if handle == _INVALID_HANDLE_VALUE or handle is None: + raise _win_error(str(path)) + try: + attributes, tag = _attribute_tag(handle, str(path)) + except OSError: + _kernel32.CloseHandle(handle) + raise + if not follow and _is_link(attributes, tag): + _kernel32.CloseHandle(handle) + raise OSError(errno.ELOOP, "Too many levels of symbolic links", str(path)) + if not attributes & _FILE_ATTRIBUTE_DIRECTORY: + _kernel32.CloseHandle(handle) + raise NotADirectoryError(errno.ENOTDIR, "Not a directory", str(path)) + return _wrap(handle, writable=False) + + def open_at(dir_fd: int, name: str, flags: int, mode: int = 0o600) -> int: + """``os.open(name, flags, mode, dir_fd=dir_fd)`` for one component. + + ``flags`` is the POSIX vocabulary: ``O_RDONLY``/``O_WRONLY``/``O_RDWR``, + ``O_CREAT``, ``O_EXCL``, plus :data:`AT_NOFOLLOW`, :data:`AT_DIRECTORY` + and :data:`AT_NONBLOCK` (accepted and ignored — nothing in an NTFS + namespace blocks an open the way a reader-less FIFO does). ``O_TRUNC`` + and ``O_APPEND`` are refused: no anchored caller uses them, and a silent + approximation is worse than a loud gap. ``mode`` has no Windows meaning + and is accepted for signature parity. + + A read-only open of a directory succeeds, as it does on POSIX, so a + caller's ``S_ISREG`` check sees the directory; a write open refuses one + with ``IsADirectoryError``, the ``EISDIR`` ``open(2)`` gives.""" + del mode + _single_component(name) + accmode = flags & (os.O_RDONLY | os.O_WRONLY | os.O_RDWR) + if flags & (os.O_TRUNC | os.O_APPEND): + raise ValueError("O_TRUNC/O_APPEND are not supported relative to a handle") + writable = accmode in (os.O_WRONLY, os.O_RDWR) + access = 0 + if accmode in (os.O_RDONLY, os.O_RDWR): + access |= _FILE_GENERIC_READ + if writable: + access |= _FILE_GENERIC_WRITE + if flags & os.O_CREAT: + disposition = _FILE_CREATE if flags & os.O_EXCL else _FILE_OPEN_IF + else: + disposition = _FILE_OPEN + options = 0 + if flags & AT_DIRECTORY: + options |= _FILE_DIRECTORY_FILE + access |= _FILE_TRAVERSE + elif writable: + options |= _FILE_NON_DIRECTORY_FILE + if flags & AT_NOFOLLOW: + options |= _FILE_OPEN_REPARSE_POINT + handle = _nt_open( + _handle_of(dir_fd), name, access=access, disposition=disposition, options=options + ) + if flags & AT_NOFOLLOW: + _refuse_link(handle, name) + return _wrap(handle, writable=writable) + + def stat_at(dir_fd: int, name: str) -> os.stat_result: + """``os.stat(name, dir_fd=dir_fd, follow_symlinks=False)``: the entry's + own metadata through a handle, never a path. A symlink or mount point + reports ``S_IFLNK`` so ``S_ISREG``/``S_ISDIR`` answer False for it, as + ``lstat`` answers on POSIX; CPython's ``fstat`` would otherwise describe + the opened reparse point as the file or directory it stands in for.""" + _single_component(name) + handle = _nt_open( + _handle_of(dir_fd), + name, + access=_FILE_READ_ATTRIBUTES, + disposition=_FILE_OPEN, + options=_FILE_OPEN_REPARSE_POINT, + ) + fd = _wrap(handle, writable=False) + try: + observed = os.fstat(fd) + attributes, tag = _attribute_tag(handle, name) + finally: + os.close(fd) + if not _is_link(attributes, tag): + return observed + link_mode = stat.S_IFLNK | stat.S_IMODE(observed.st_mode) + return os.stat_result( + ( + link_mode, + observed.st_ino, + observed.st_dev, + observed.st_nlink, + observed.st_uid, + observed.st_gid, + observed.st_size, + int(observed.st_atime), + int(observed.st_mtime), + int(observed.st_ctime), + ), + { + "st_atime": observed.st_atime, + "st_mtime": observed.st_mtime, + "st_ctime": observed.st_ctime, + "st_atime_ns": observed.st_atime_ns, + "st_mtime_ns": observed.st_mtime_ns, + "st_ctime_ns": observed.st_ctime_ns, + "st_file_attributes": attributes, + "st_reparse_tag": tag, + }, + ) + + def _rename_information(flags: int, root_handle: int, name: str) -> bytes: + # FILE_RENAME_INFORMATION: union{BOOLEAN ReplaceIfExists; ULONG Flags}; + # ; HANDLE RootDirectory; ULONG FileNameLength; WCHAR FileName[]. + # A ULONG 1 in the union reads as ReplaceIfExists=TRUE for the classic + # class and as REPLACE_IF_EXISTS for the Ex one, so one buffer serves both. + encoded = name.encode("utf-16-le") + return struct.pack(" int: + buf = ctypes.create_string_buffer(payload, len(payload)) + iosb = _IO_STATUS_BLOCK() + return int( + _ntdll.NtSetInformationFile(handle, ctypes.byref(iosb), buf, len(payload), info_class) + ) + + def replace_at(src_dir_fd: int, src: str, dst_dir_fd: int, dst: str) -> None: + """``os.replace(src, dst, src_dir_fd=..., dst_dir_fd=...)``: rename the + entry ``src`` names under ``src_dir_fd`` onto ``dst`` under ``dst_dir_fd``, + replacing an existing ``dst``. The source entry is renamed AS the entry + (a link at ``src`` moves as a link, never its target), and the rename is + performed on that entry's own handle — nothing resolves a path. + + ``FileRenameInformationEx`` with POSIX semantics first (Windows 10 1709+, + NTFS/ReFS): the old ``dst`` file is unlinked from the namespace even while + another handle keeps it open, which is what ``rename(2)`` does. Where the + kernel or volume refuses the class, the classic ``FileRenameInformation`` + with ``ReplaceIfExists`` is used, whose replace fails with a sharing + violation while ``dst`` is open elsewhere — the retry the path-based + writer already makes applies to both.""" + _single_component(src) + _single_component(dst) + handle = _nt_open( + _handle_of(src_dir_fd), + src, + access=_DELETE, + disposition=_FILE_OPEN, + options=_FILE_OPEN_REPARSE_POINT, + ) + try: + root = _handle_of(dst_dir_fd) + status = _set_information( + handle, + _FileRenameInformationEx, + _rename_information( + _FILE_RENAME_REPLACE_IF_EXISTS | _FILE_RENAME_POSIX_SEMANTICS, root, dst + ), + dst, + ) + if status & 0xFFFFFFFF in _EX_CLASS_UNSUPPORTED: + status = _set_information( + handle, + _FileRenameInformation, + _rename_information(_FILE_RENAME_REPLACE_IF_EXISTS, root, dst), + dst, + ) + if status < 0: + raise _nt_error(status, dst) + finally: + _kernel32.CloseHandle(handle) + + def unlink_at(dir_fd: int, name: str) -> None: + """``os.unlink(name, dir_fd=dir_fd)``: delete the entry itself (a link is + removed, not followed) through its own handle.""" + _single_component(name) + handle = _nt_open( + _handle_of(dir_fd), + name, + access=_DELETE, + disposition=_FILE_OPEN, + options=_FILE_OPEN_REPARSE_POINT | _FILE_NON_DIRECTORY_FILE, + ) + try: + status = _set_information( + handle, + _FileDispositionInformationEx, + struct.pack(" int: + del path, follow + raise _unsupported() + + def open_at(dir_fd: int, name: str, flags: int, mode: int = 0o600) -> int: + del dir_fd, name, flags, mode + raise _unsupported() + + def stat_at(dir_fd: int, name: str) -> os.stat_result: + del dir_fd, name + raise _unsupported() + + def replace_at(src_dir_fd: int, src: str, dst_dir_fd: int, dst: str) -> None: + del src_dir_fd, src, dst_dir_fd, dst + raise _unsupported() + + def unlink_at(dir_fd: int, name: str) -> None: + del dir_fd, name + raise _unsupported() diff --git a/tests/test_artifact_publication.py b/tests/test_artifact_publication.py index 0e0350d3..959dbcd2 100644 --- a/tests/test_artifact_publication.py +++ b/tests/test_artifact_publication.py @@ -1900,6 +1900,7 @@ def edit_during_fsync(fd): destination.write_bytes(b"operator during fsync") if fallback: + monkeypatch.setattr(platform_util, "HANDLE_ANCHORED_WRITES", False) monkeypatch.setattr(platform_util, "DIR_FD_ANCHORED_WRITES", False) monkeypatch.setattr(publication, "DIR_FD_ANCHORED_WRITES", False) monkeypatch.setattr(os, "fsync", edit_during_fsync) diff --git a/tests/test_engine.py b/tests/test_engine.py index b5df3c23..fcbf400c 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -114,14 +114,14 @@ QUIET = NotifyPolicy(desktop=False, file=True) # DW-310: attempt-owned spec restoration and lifecycle normalization refuse -# before any write on a host without descriptor-relative writes (Windows) and -# pause for manual adoption, so a rollback that has to put the bound spec back -# cannot converge there by design. Mirrors `tests/test_recovery_flow.py`, which -# pins that refusal directly; these rows assert the descriptor-capable -# convergence and stay POSIX-only. +# before any write on a host without handle-anchored writes and pause for +# manual adoption, so a rollback that has to put the bound spec back cannot +# converge there by design. Both real hosts have an arm — POSIX `dir_fd`, +# Windows `platform_util.win32_at` — so these rows run on both; the marker +# mirrors `tests/test_recovery_flow.py`'s and only a host with neither skips. requires_descriptor_restoration = pytest.mark.skipif( - not platform_util.DIR_FD_ANCHORED_WRITES, - reason="automatic owned-spec restore requires descriptor-relative writes (DW-310)", + not platform_util.HANDLE_ANCHORED_WRITES, + reason="automatic owned-spec restore requires handle-anchored writes (DW-310)", ) diff --git a/tests/test_platform_util.py b/tests/test_platform_util.py index 125dd85d..45d94417 100644 --- a/tests/test_platform_util.py +++ b/tests/test_platform_util.py @@ -1837,9 +1837,16 @@ def test_walk_files_unlinked_refuses_a_link_like_top(tmp_path, monkeypatch): DIR_FD = pytest.mark.skipif( not platform_util.DIR_FD_ANCHORED_WRITES, reason="dir-fd anchoring is POSIX-only" ) +# The rows below that need only what BOTH anchored arms provide — a confined +# directory handle, the ``*_at`` writers, the confined writers — run on Windows +# too, through ``win32_at``; ``DIR_FD`` stays on the rows that also need +# ``scandir(fd)``, mode bits, or a symlink the runner may not be allowed to plant. +ANCHORED = pytest.mark.skipif( + not platform_util.HANDLE_ANCHORED_WRITES, reason="needs a handle-anchored write arm" +) -@DIR_FD +@ANCHORED def test_open_dir_confined_returns_a_descriptor_for_a_clean_chain(tmp_path): """The positive control the refusals below need: an unplanted chain hands back a descriptor for the directory that was asked for, not merely a non-None int. @@ -1875,7 +1882,7 @@ def test_open_dir_confined_refuses_a_symlinked_component(tmp_path): assert platform_util.open_dir_confined(root, root / ".bmad-loop" / "runs") is None -@DIR_FD +@ANCHORED def test_open_dir_confined_refuses_a_target_outside_the_root(tmp_path): """Confinement is refused before a single directory is opened: a target that is not under the root has no chain to walk, however clean its own ancestry is.""" @@ -1887,7 +1894,7 @@ def test_open_dir_confined_refuses_a_target_outside_the_root(tmp_path): assert platform_util.open_dir_confined(root, elsewhere) is None -@DIR_FD +@ANCHORED def test_open_dir_confined_refuses_a_missing_component(tmp_path): """An absent directory is refused rather than created. The confined writers lean on this: they require the parent to EXIST, because a walk cannot vouch @@ -2017,8 +2024,13 @@ def test_open_dir_confined_search_only_crosses_execute_only_intermediate_directo @contextmanager def _dir_fd(directory: Path): - """The descriptor the anchored helpers take, closed on the way out.""" - fd = os.open(directory, os.O_RDONLY | os.O_DIRECTORY) + """The descriptor the anchored helpers take, closed on the way out — the + ``O_DIRECTORY`` open on POSIX, a directory handle through ``win32_at`` on + Windows, exactly as `open_dir_confined` anchors on each arm.""" + if platform_util.DIR_FD_ANCHORED_WRITES: + fd = os.open(directory, os.O_RDONLY | os.O_DIRECTORY) + else: + fd = platform_util.win32_at.open_directory(directory) try: yield fd finally: @@ -2070,7 +2082,7 @@ def test_atomic_write_bytes_at_lands_a_private_mode(tmp_path): assert stat.S_IMODE(landed.stat().st_mode) == 0o600, oct(landed.stat().st_mode) -@DIR_FD +@ANCHORED def test_atomic_write_bytes_at_preserves_crlf_verbatim(tmp_path): """The reason a bytes-anchored variant had to exist at all (#593). @@ -2095,7 +2107,7 @@ def test_atomic_write_bytes_at_preserves_crlf_verbatim(tmp_path): assert (tmp_path / "spec.md").read_bytes() == payload -@DIR_FD +@ANCHORED def test_atomic_write_text_at_removes_its_temp_when_the_write_fails(tmp_path, monkeypatch): """A failed anchored write leaves the directory as it found it. @@ -2117,7 +2129,7 @@ def boom(fd): assert list(tmp_path.glob("*.tmp")) == [] # and nothing was stranded beside it -@DIR_FD +@ANCHORED def test_atomic_write_bytes_at_removes_its_temp_when_the_write_fails(tmp_path, monkeypatch): """The bytes arm's own cleanup pin — mirrored, per the banner's reasoning.""" target = tmp_path / "policy.toml" @@ -2135,7 +2147,7 @@ def boom(fd): assert list(tmp_path.glob("*.tmp")) == [] -@DIR_FD +@ANCHORED def test_atomic_write_bytes_at_writable_refusal_precedes_staging(tmp_path, monkeypatch): target = tmp_path / "owned.md" target.write_bytes(b"before") @@ -2163,7 +2175,7 @@ def unexpected_stage(*_args, **_kwargs): assert target.read_bytes() == b"before" -@DIR_FD +@ANCHORED def test_atomic_write_bytes_at_does_not_verify_after_prepublication_failure(tmp_path): target = tmp_path / "owned.md" target.write_bytes(b"before") @@ -2330,6 +2342,24 @@ def test_atomic_write_text_confined_writes_a_clean_tree(tmp_path): assert (parent / "policy.toml").read_text(encoding="utf-8") == "x = 1\n" +def test_atomic_write_text_confined_lands_the_platform_line_ending(tmp_path): + """The confined TEXT writer keeps `atomic_write_text`'s translating newline + default on every arm — LF on POSIX, CRLF on Windows — so anchoring the write + through `win32_at` changed no byte a ledger or decisions file lands there. + `atomic_write_text_at` (records, POSIX-only callers) is the one that never + translates; the confined writer is what the operator-edited cohort uses. + + Ablation: drop the `newline=None` the confined arm passes `_atomic_write_at` + and this reddens on Windows with LF where the path writer landed CRLF.""" + root = tmp_path / "project" + parent = root / ".bmad-loop" + parent.mkdir(parents=True) + + platform_util.atomic_write_text_confined(parent / "ledger.md", "a\nb\n", confine_root=root) + + assert (parent / "ledger.md").read_bytes() == f"a{os.linesep}b{os.linesep}".encode() + + def test_atomic_write_confined_refuses_a_parent_ref_below_the_root(tmp_path): """`is_relative_to` is a lexical PREFIX test, so `root/specs/../../outside/f` passes it while naming a path outside the root — and `..` is a real directory @@ -2379,6 +2409,7 @@ def test_atomic_write_confined_refuses_a_parent_ref_on_the_fallback_arm(tmp_path Ablation: delete the `has_parent_ref` gate and this fails `DID NOT RAISE`, with the payload landing over `outside/victim.md` through the plain no-follow write.""" + monkeypatch.setattr(platform_util, "HANDLE_ANCHORED_WRITES", False) monkeypatch.setattr(platform_util, "DIR_FD_ANCHORED_WRITES", False) root = tmp_path / "project" (root / "specs").mkdir(parents=True) @@ -2469,6 +2500,7 @@ def test_create_exclusive_confined_refuses_a_symlinked_parent(tmp_path, monkeypa platform_util.create_exclusive_confined(target, confine_root=root) assert list(outside.iterdir()) == [] # nothing landed outside + monkeypatch.setattr(platform_util, "HANDLE_ANCHORED_WRITES", False) monkeypatch.setattr(platform_util, "DIR_FD_ANCHORED_WRITES", False) with pytest.raises(platform_util.UnconfinedWriteError, match="without a redirect"): platform_util.create_exclusive_confined(target, confine_root=root) @@ -2577,7 +2609,7 @@ def test_atomic_write_bytes_confined_preserves_crlf_verbatim(tmp_path): assert (parent / "story.md").read_bytes() == payload -@DIR_FD +@ANCHORED def test_atomic_write_bytes_confined_runs_callback_with_live_published_fd(tmp_path): root = tmp_path / "project" parent = root / "specs" @@ -2596,11 +2628,13 @@ def verify_after_publish(published_fd: int | None) -> None: ) assert observed and observed[0][1] == b"published" - with pytest.raises(OSError, match="Bad file descriptor"): - os.fstat(observed[0][0]) + with pytest.raises(OSError) as closed: # EBADF: "Bad file descriptor" on POSIX, + os.fstat(observed[0][0]) # "Invalid handle" on Windows + assert closed.value.errno == errno.EBADF def test_atomic_write_bytes_confined_fallback_post_callback_receives_no_fd(tmp_path, monkeypatch): + monkeypatch.setattr(platform_util, "HANDLE_ANCHORED_WRITES", False) monkeypatch.setattr(platform_util, "DIR_FD_ANCHORED_WRITES", False) root = tmp_path / "project" parent = root / "specs" @@ -2620,6 +2654,7 @@ def verify_after_publish(dir_fd: int | None) -> None: def test_confined_fallback_callback_failure_does_not_unlink_reused_temp_name(tmp_path, monkeypatch): + monkeypatch.setattr(platform_util, "HANDLE_ANCHORED_WRITES", False) monkeypatch.setattr(platform_util, "DIR_FD_ANCHORED_WRITES", False) root = tmp_path / "project" parent = root / "specs" @@ -2648,7 +2683,7 @@ def fail_after_publish(dir_fd: int | None) -> None: assert reused.read_bytes() == b"another writer" -@DIR_FD +@ANCHORED def test_confined_anchored_callback_failure_does_not_unlink_reused_temp_name(tmp_path, monkeypatch): root = tmp_path / "project" parent = root / "specs" @@ -2658,12 +2693,7 @@ def test_confined_anchored_callback_failure_does_not_unlink_reused_temp_name(tmp failure = RuntimeError("post-publication verification failed") def fixed_temp(dir_fd: int, _prefix: str, _name: str) -> tuple[int, str]: - fd = os.open( - reused.name, - os.O_WRONLY | os.O_CREAT | os.O_EXCL, - 0o600, - dir_fd=dir_fd, - ) + fd = platform_util.open_at(dir_fd, reused.name, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o600) return fd, reused.name published_fds: list[int] = [] @@ -2684,8 +2714,9 @@ def fail_after_publish(published_fd: int | None) -> None: assert excinfo.value is failure assert target.read_bytes() == b"published" assert reused.read_bytes() == b"another writer" - with pytest.raises(OSError, match="Bad file descriptor"): - os.fstat(published_fds[0]) + with pytest.raises(OSError) as closed: # EBADF: "Bad file descriptor" on POSIX, + os.fstat(published_fds[0]) # "Invalid handle" on Windows + assert closed.value.errno == errno.EBADF @DIR_FD @@ -2733,6 +2764,7 @@ def test_atomic_write_text_confined_falls_back_without_dir_fd(tmp_path, monkeypa Ablation: delete the `path_is_confined` check and this fails `DID NOT RAISE`, with the file landing in `outside/` exactly as the unguarded POSIX path did.""" + monkeypatch.setattr(platform_util, "HANDLE_ANCHORED_WRITES", False) monkeypatch.setattr(platform_util, "DIR_FD_ANCHORED_WRITES", False) root = tmp_path / "project" root.mkdir() @@ -2759,6 +2791,7 @@ def test_atomic_write_text_confined_falls_back_without_dir_fd(tmp_path, monkeypa def test_confined_fallback_rejects_before_writable_probe_and_staging(tmp_path, monkeypatch): + monkeypatch.setattr(platform_util, "HANDLE_ANCHORED_WRITES", False) monkeypatch.setattr(platform_util, "DIR_FD_ANCHORED_WRITES", False) root = tmp_path / "project" parent = root / "specs" diff --git a/tests/test_portability_guard.py b/tests/test_portability_guard.py index 80a69d41..30a05598 100644 --- a/tests/test_portability_guard.py +++ b/tests/test_portability_guard.py @@ -296,6 +296,7 @@ ("resolve.py", "_reject_json_constant"), ("runs.py", "_refuse_live_session"), ("runs.py", "_refuse_uncontained_run_dir"), + ("win32_at.py", "_refuse_link"), # ELOOP for a symlink/junction under O_NOFOLLOW ("workspace.py", "_refuse_foreign_checkout"), ("worktree_flow.py", "_refuse_integrated_artifacts"), ("worktree_flow.py", "_refuse_refused_residue"), diff --git a/tests/test_recovery_flow.py b/tests/test_recovery_flow.py index 2bfca1bd..52b88e3e 100644 --- a/tests/test_recovery_flow.py +++ b/tests/test_recovery_flow.py @@ -34,9 +34,33 @@ QUIET = NotifyPolicy(desktop=False, file=True) requires_descriptor_restoration = pytest.mark.skipif( - not platform_util.DIR_FD_ANCHORED_WRITES, - reason="automatic restore requires descriptor-relative writes", + not platform_util.HANDLE_ANCHORED_WRITES, + reason="automatic restore requires handle-anchored writes", ) +# Rows that rename a directory OUT FROM UNDER a writer holding a handle beneath it. +# Windows refuses that rename outright (ERROR_ACCESS_DENIED while any handle is +# open below the directory), so the swap these rows stage cannot be performed +# there — the OS closes the race before the anchored writer has to. The refusal +# itself is pinned on the win32 arm by tests/test_win32_at.py. +posix_parent_swap_under_writer = pytest.mark.skipif( + sys.platform == "win32", + reason="Windows refuses to rename a directory with a handle open beneath it", +) + + +def _plant_directory_redirect(link: Path, target: Path) -> None: + """Plant a directory redirect at ``link`` the confined walk must refuse. + + A symlink on POSIX; on Windows a JUNCTION, which needs no elevation where a + directory symlink needs SeCreateSymbolicLinkPrivilege — and is the redirect + an unprivileged session can actually plant, so it is the one worth pinning + against the ``win32_at`` walk.""" + if sys.platform == "win32": + import _winapi + + _winapi.CreateJunction(str(target), str(link)) + else: + link.symlink_to(target, target_is_directory=True) def _policy(**scm) -> Policy: @@ -102,7 +126,8 @@ def test_owned_spec_restore_recreates_missing_canonical_parents(tmp_path): def test_owned_spec_restore_forced_fallback_refuses_before_path_writer(tmp_path, monkeypatch): - monkeypatch.setattr(recovery_flow, "DIR_FD_ANCHORED_WRITES", False) + monkeypatch.setattr(recovery_flow, "HANDLE_ANCHORED_WRITES", False) + monkeypatch.setattr(platform_util, "HANDLE_ANCHORED_WRITES", False) monkeypatch.setattr(platform_util, "DIR_FD_ANCHORED_WRITES", False) spec = tmp_path.resolve() / "owned.md" original = b"operator bytes\n" @@ -125,7 +150,8 @@ def path_writer_is_forbidden(path, *_args, **_kwargs): def test_owned_spec_restore_forced_fallback_does_not_create_missing_parents(tmp_path, monkeypatch): - monkeypatch.setattr(recovery_flow, "DIR_FD_ANCHORED_WRITES", False) + monkeypatch.setattr(recovery_flow, "HANDLE_ANCHORED_WRITES", False) + monkeypatch.setattr(platform_util, "HANDLE_ANCHORED_WRITES", False) monkeypatch.setattr(platform_util, "DIR_FD_ANCHORED_WRITES", False) first_missing_parent = tmp_path.resolve() / "new" spec = first_missing_parent / "deep" / "owned.md" @@ -143,7 +169,8 @@ def path_writer_is_forbidden(*_args, **_kwargs): def test_owned_spec_normalization_forced_fallback_refuses_before_writer(tmp_path, monkeypatch): - monkeypatch.setattr(recovery_flow, "DIR_FD_ANCHORED_WRITES", False) + monkeypatch.setattr(recovery_flow, "HANDLE_ANCHORED_WRITES", False) + monkeypatch.setattr(platform_util, "HANDLE_ANCHORED_WRITES", False) monkeypatch.setattr(platform_util, "DIR_FD_ANCHORED_WRITES", False) spec = tmp_path.resolve() / "owned.md" original = b"---\nstatus: in-progress\n---\n\noperator bytes\n" @@ -168,12 +195,19 @@ def test_owned_spec_normalization_forced_fallback_refuses_before_writer(tmp_path @pytest.mark.skipif(sys.platform != "win32", reason="native Windows coverage") -def test_owned_spec_restore_native_windows_refuses_before_path_writer(tmp_path, monkeypatch): - assert not recovery_flow.DIR_FD_ANCHORED_WRITES +def test_owned_spec_restore_native_windows_anchors_at_a_handle(tmp_path, monkeypatch): + """Windows restores through the handle-relative arm, not the path writer. + + DW-309/310 first made this host fail closed because CPython offers no + `dir_fd` there; `platform_util.win32_at` now supplies the same anchor through + NT handle-relative opens, so the row that pinned the refusal pins the + restoration instead — and that the generic confined PATH writer is still + never reached, which is what the refusal existed to guarantee.""" assert not platform_util.DIR_FD_ANCHORED_WRITES + assert recovery_flow.HANDLE_ANCHORED_WRITES spec = tmp_path.resolve() / "owned.md" - original = b"operator bytes\n" - spec.write_bytes(original) + spec.write_bytes(b"operator bytes\n") + snapshot = b"---\nstatus: ready-for-dev\n---\n\noperator input\n" path_writer_calls: list[Path] = [] def path_writer_is_forbidden(path, *_args, **_kwargs): @@ -181,12 +215,21 @@ def path_writer_is_forbidden(path, *_args, **_kwargs): raise AssertionError("generic confined writer was called") monkeypatch.setattr(platform_util, "_atomic_write_confined", path_writer_is_forbidden) + real_write = platform_util.atomic_write_bytes_at + anchored: list[str] = [] - with pytest.raises(_OwnedSpecAuthorityError, match="restoration is unavailable"): - RecoveryFlow._restore_attempt_owned_spec_bytes(spec, b"snapshot bytes\n") + def spy(dir_fd, name, data, **kwargs): + anchored.append(name) + return real_write(dir_fd, name, data, **kwargs) + + monkeypatch.setattr(recovery_flow, "atomic_write_bytes_at", spy) + + RecoveryFlow._restore_attempt_owned_spec_bytes(spec, snapshot) + assert anchored == ["owned.md"] assert path_writer_calls == [] - assert spec.read_bytes() == original + assert spec.read_bytes() == snapshot + assert list(tmp_path.glob("*.tmp")) == [] @pytest.mark.parametrize("resolve_fault", NUL_PATH_RESOLVE_FAULTS) @@ -320,7 +363,7 @@ def fail_write(*_args, **_kwargs): @pytest.mark.parametrize("failure", NUL_PATH_RESOLVE_FAULTS) @pytest.mark.skipif( - not platform_util.DIR_FD_ANCHORED_WRITES, reason="dir-fd anchoring is POSIX-only" + not platform_util.HANDLE_ANCHORED_WRITES, reason="needs a handle-anchored write arm" ) def test_owned_spec_restore_does_not_translate_content_read_value_error( tmp_path, monkeypatch, failure @@ -351,8 +394,9 @@ def test_owned_spec_restore_preserves_byte_hostile_snapshot(tmp_path): assert spec.read_bytes() == snapshot +@posix_parent_swap_under_writer @pytest.mark.skipif( - not platform_util.DIR_FD_ANCHORED_WRITES, reason="dir-fd anchoring is POSIX-only" + not platform_util.HANDLE_ANCHORED_WRITES, reason="needs a handle-anchored write arm" ) @pytest.mark.parametrize("victim_matches", [False, True], ids=["different-victim", "equal-victim"]) def test_owned_spec_restore_parent_swap_before_publication_stays_anchored( @@ -374,7 +418,7 @@ def test_owned_spec_restore_parent_swap_before_publication_stays_anchored( def swap_before_replace(dir_fd, name, data, **kwargs): def swap() -> None: parent.rename(moved) - parent.symlink_to(outside, target_is_directory=True) + _plant_directory_redirect(parent, outside) kwargs["_before_replace"] = swap return real_write(dir_fd, name, data, **kwargs) @@ -386,11 +430,11 @@ def swap() -> None: assert victim.read_bytes() == victim_before assert (moved / spec.name).read_bytes() == snapshot - assert parent.is_symlink() + assert platform_util.is_link_like(parent) @pytest.mark.skipif( - not platform_util.DIR_FD_ANCHORED_WRITES, reason="dir-fd anchoring is POSIX-only" + not platform_util.HANDLE_ANCHORED_WRITES, reason="needs a handle-anchored write arm" ) def test_owned_spec_restore_refuses_parent_swap_before_filesystem_root_walk(tmp_path, monkeypatch): parent = tmp_path.resolve() / "artifacts" @@ -411,7 +455,7 @@ def swap_before_walk(root: Path, target: Path, **kwargs): if not swapped: swapped = True parent.rename(moved) - parent.symlink_to(outside, target_is_directory=True) + _plant_directory_redirect(parent, outside) return real_open(root, target, **kwargs) monkeypatch.setattr(recovery_flow, "open_dir_confined", swap_before_walk) @@ -423,8 +467,9 @@ def swap_before_walk(root: Path, target: Path, **kwargs): assert (moved / spec.name).read_bytes() == original +@posix_parent_swap_under_writer @pytest.mark.skipif( - not platform_util.DIR_FD_ANCHORED_WRITES, reason="dir-fd anchoring is POSIX-only" + not platform_util.HANDLE_ANCHORED_WRITES, reason="needs a handle-anchored write arm" ) def test_owned_spec_restore_ancestor_swap_before_publication_stays_anchored(tmp_path, monkeypatch): ancestor = tmp_path.resolve() / "artifacts" @@ -442,7 +487,7 @@ def test_owned_spec_restore_ancestor_swap_before_publication_stays_anchored(tmp_ def swap_before_replace(dir_fd, name, data, **kwargs): def swap() -> None: ancestor.rename(moved) - ancestor.symlink_to(outside, target_is_directory=True) + _plant_directory_redirect(ancestor, outside) kwargs["_before_replace"] = swap return real_write(dir_fd, name, data, **kwargs) @@ -457,7 +502,7 @@ def swap() -> None: @pytest.mark.skipif( - not platform_util.DIR_FD_ANCHORED_WRITES, reason="dir-fd anchoring is POSIX-only" + not platform_util.HANDLE_ANCHORED_WRITES, reason="needs a handle-anchored write arm" ) def test_owned_spec_restore_detects_parent_swap_after_first_callback_check(tmp_path, monkeypatch): parent = tmp_path.resolve() / "artifacts" @@ -478,7 +523,7 @@ def swap_after_first_probe(root: Path, target: Path, **kwargs): probes += 1 if probes == 1: parent.rename(moved) - parent.symlink_to(outside, target_is_directory=True) + _plant_directory_redirect(parent, outside) return fd monkeypatch.setattr(recovery_flow, "open_dir_confined", swap_after_first_probe) @@ -492,12 +537,25 @@ def swap_after_first_probe(root: Path, target: Path, **kwargs): @pytest.mark.skipif( - not platform_util.DIR_FD_ANCHORED_WRITES, reason="dir-fd anchoring is POSIX-only" + not platform_util.HANDLE_ANCHORED_WRITES, reason="needs a handle-anchored write arm" +) +@pytest.mark.parametrize( + "replacement", ["missing", "symlink", "junction", "fifo", "socket", "directory"] ) -@pytest.mark.parametrize("replacement", ["missing", "symlink", "fifo", "socket", "directory"]) def test_owned_spec_restore_types_final_entry_substitution_as_authority_loss( tmp_path, monkeypatch, replacement ): + """Every shape the name can take after publication is authority loss, on + both anchored arms. ``fifo``/``socket`` are POSIX entry types; ``symlink`` at + a FILE name needs elevation on Windows, where ``junction`` is the redirect an + unprivileged writer plants instead (a directory reparse point the win32 arm + must refuse as it refuses a link) and has no POSIX counterpart.""" + if replacement in {"fifo", "socket"} and sys.platform == "win32": + pytest.skip(f"{replacement} is a POSIX entry type") + if replacement == "symlink" and sys.platform == "win32": + pytest.skip("file symlink creation may need elevation") + if replacement == "junction" and sys.platform != "win32": + pytest.skip("junctions are a Windows reparse point") parent = tmp_path.resolve() / "artifacts" parent.mkdir() spec = parent / "owned.md" @@ -519,6 +577,8 @@ def replace_then_verify(published_fd): spec.unlink() if replacement == "symlink": spec.symlink_to(victim) + elif replacement == "junction": + _plant_directory_redirect(spec, outside) elif replacement == "fifo": os.mkfifo(spec) elif replacement == "socket": @@ -545,6 +605,8 @@ def replace_then_verify(published_fd): assert victim.read_bytes() == b"external victim" if replacement == "symlink": assert spec.is_symlink() + elif replacement == "junction": + assert platform_util.is_link_like(spec) elif replacement == "fifo": assert stat.S_ISFIFO(spec.lstat().st_mode) elif replacement == "socket": @@ -556,7 +618,7 @@ def replace_then_verify(published_fd): @pytest.mark.skipif( - not platform_util.DIR_FD_ANCHORED_WRITES, reason="dir-fd anchoring is POSIX-only" + not platform_util.HANDLE_ANCHORED_WRITES, reason="needs a handle-anchored write arm" ) def test_owned_spec_restore_anchored_snapshot_plus_suffix_is_a_genuine_mismatch( tmp_path, monkeypatch @@ -586,7 +648,7 @@ def bounded_read(fd: int, size: int) -> bytes: @pytest.mark.skipif( - not platform_util.DIR_FD_ANCHORED_WRITES, reason="dir-fd anchoring is POSIX-only" + not platform_util.HANDLE_ANCHORED_WRITES, reason="needs a handle-anchored write arm" ) def test_owned_spec_restore_preserves_raw_anchored_os_read_failure(tmp_path, monkeypatch): spec = tmp_path.resolve() / "owned.md" @@ -606,7 +668,7 @@ def fail_read(_fd, _size): @pytest.mark.skipif( - not platform_util.DIR_FD_ANCHORED_WRITES, reason="dir-fd anchoring is POSIX-only" + not platform_util.HANDLE_ANCHORED_WRITES, reason="needs a handle-anchored write arm" ) def test_owned_spec_restore_preserves_raw_postpublication_os_read_failure(tmp_path, monkeypatch): spec = tmp_path.resolve() / "owned.md" @@ -644,7 +706,7 @@ def fail_after_publish(fd: int, size: int) -> bytes: @pytest.mark.skipif( - not platform_util.DIR_FD_ANCHORED_WRITES, reason="dir-fd anchoring is POSIX-only" + not platform_util.HANDLE_ANCHORED_WRITES, reason="needs a handle-anchored write arm" ) def test_owned_spec_restore_anchored_uses_filesystem_anchor_for_external_target( tmp_path, monkeypatch @@ -669,7 +731,7 @@ def record_root(root, target, **kwargs): @pytest.mark.skipif( - not platform_util.DIR_FD_ANCHORED_WRITES, reason="dir-fd anchoring is POSIX-only" + not platform_util.HANDLE_ANCHORED_WRITES, reason="needs a handle-anchored write arm" ) def test_owned_spec_restore_preserves_anchored_writable_target_refusal(tmp_path, monkeypatch): spec = tmp_path.resolve() / "owned.md" @@ -690,7 +752,7 @@ def refuse(_dir_fd, _name): @pytest.mark.skipif( - not platform_util.DIR_FD_ANCHORED_WRITES, reason="dir-fd anchoring is POSIX-only" + not platform_util.HANDLE_ANCHORED_WRITES, reason="needs a handle-anchored write arm" ) def test_owned_spec_restore_anchored_refuses_missing_target_appearing_before_staging( tmp_path, monkeypatch @@ -747,7 +809,7 @@ def _replace_target(spec: Path, data: bytes) -> None: @pytest.mark.skipif( - not platform_util.DIR_FD_ANCHORED_WRITES, reason="dir-fd anchoring is POSIX-only" + not platform_util.HANDLE_ANCHORED_WRITES, reason="needs a handle-anchored write arm" ) def test_owned_spec_restore_refuses_target_replacement_after_staging(tmp_path, monkeypatch): parent = tmp_path.resolve() / "artifacts" @@ -778,7 +840,7 @@ def replace_then_validate() -> None: @pytest.mark.skipif( - not platform_util.DIR_FD_ANCHORED_WRITES, reason="dir-fd anchoring is POSIX-only" + not platform_util.HANDLE_ANCHORED_WRITES, reason="needs a handle-anchored write arm" ) def test_owned_spec_restore_refuses_in_place_target_edit_after_staging(tmp_path, monkeypatch): parent = tmp_path.resolve() / "artifacts" @@ -813,7 +875,7 @@ def edit_then_validate() -> None: @pytest.mark.skipif( - not platform_util.DIR_FD_ANCHORED_WRITES, reason="dir-fd anchoring is POSIX-only" + not platform_util.HANDLE_ANCHORED_WRITES, reason="needs a handle-anchored write arm" ) def test_owned_spec_restore_refuses_in_place_edit_during_initial_content_read( tmp_path, monkeypatch @@ -846,7 +908,7 @@ def mutate_then_read(fd: int, size: int) -> bytes: @pytest.mark.skipif( - not platform_util.DIR_FD_ANCHORED_WRITES, reason="dir-fd anchoring is POSIX-only" + not platform_util.HANDLE_ANCHORED_WRITES, reason="needs a handle-anchored write arm" ) def test_owned_spec_restore_refuses_short_initial_content_sample(tmp_path, monkeypatch): parent = tmp_path.resolve() / "artifacts" @@ -864,7 +926,7 @@ def test_owned_spec_restore_refuses_short_initial_content_sample(tmp_path, monke @pytest.mark.skipif( - not platform_util.DIR_FD_ANCHORED_WRITES, reason="dir-fd anchoring is POSIX-only" + not platform_util.HANDLE_ANCHORED_WRITES, reason="needs a handle-anchored write arm" ) def test_owned_spec_restore_compares_bounded_multichunk_content_after_staging( tmp_path, monkeypatch @@ -914,7 +976,7 @@ def edit_then_validate() -> None: @pytest.mark.skipif( - not platform_util.DIR_FD_ANCHORED_WRITES, reason="dir-fd anchoring is POSIX-only" + not platform_util.HANDLE_ANCHORED_WRITES, reason="needs a handle-anchored write arm" ) def test_owned_spec_restore_refuses_name_replacement_during_prepublication_read( tmp_path, monkeypatch @@ -958,7 +1020,7 @@ def replace_then_read(fd: int, size: int) -> bytes: @pytest.mark.skipif( - not platform_util.DIR_FD_ANCHORED_WRITES, reason="dir-fd anchoring is POSIX-only" + not platform_util.HANDLE_ANCHORED_WRITES, reason="needs a handle-anchored write arm" ) def test_owned_spec_restore_preserves_raw_content_comparison_failure(tmp_path, monkeypatch): parent = tmp_path.resolve() / "artifacts" @@ -993,7 +1055,7 @@ def fail_read(_fd: int, _size: int) -> bytes: @pytest.mark.skipif( - not platform_util.DIR_FD_ANCHORED_WRITES, reason="dir-fd anchoring is POSIX-only" + not platform_util.HANDLE_ANCHORED_WRITES, reason="needs a handle-anchored write arm" ) def test_owned_spec_restore_rejects_equal_byte_final_entry_replacement(tmp_path, monkeypatch): parent = tmp_path.resolve() / "artifacts" @@ -1023,7 +1085,7 @@ def replace_then_verify(published_fd: int) -> None: @pytest.mark.skipif( - not platform_util.DIR_FD_ANCHORED_WRITES, reason="dir-fd anchoring is POSIX-only" + not platform_util.HANDLE_ANCHORED_WRITES, reason="needs a handle-anchored write arm" ) def test_owned_spec_restore_rejects_live_name_replacement_during_readback(tmp_path, monkeypatch): spec = tmp_path.resolve() / "owned.md" @@ -1064,7 +1126,7 @@ def replace_name_then_read(fd: int, size: int) -> bytes: @pytest.mark.skipif( - not platform_util.DIR_FD_ANCHORED_WRITES, reason="dir-fd anchoring is POSIX-only" + not platform_util.HANDLE_ANCHORED_WRITES, reason="needs a handle-anchored write arm" ) def test_owned_spec_restore_rejects_in_place_mutation_during_readback(tmp_path, monkeypatch): spec = tmp_path.resolve() / "owned.md" @@ -1106,8 +1168,9 @@ def mutate_then_read(fd: int, size: int) -> bytes: assert spec.read_bytes() == snapshot + b"x" +@posix_parent_swap_under_writer @pytest.mark.skipif( - not platform_util.DIR_FD_ANCHORED_WRITES, reason="dir-fd anchoring is POSIX-only" + not platform_util.HANDLE_ANCHORED_WRITES, reason="needs a handle-anchored write arm" ) def test_owned_spec_restore_real_parent_replacement_stays_in_retained_directory( tmp_path, monkeypatch @@ -1288,7 +1351,7 @@ def _assert_owned_spec_manual_adoption_pause( "spec": str(spec.resolve()), "problem": ( f"safe automatic restoration is unavailable {stage} because " - "this platform lacks descriptor-relative writes" + "this platform lacks handle-anchored writes" f"{status_guidance}; manual adoption is required" ), } @@ -1607,7 +1670,8 @@ def test_plain_owned_spec_forced_fallback_pauses_while_undoing_lifecycle_repair( flow = _make_flow( workspace=Workspace.default(project), policy=_policy(rollback_on_failure=False) ) - monkeypatch.setattr(recovery_flow, "DIR_FD_ANCHORED_WRITES", False) + monkeypatch.setattr(recovery_flow, "HANDLE_ANCHORED_WRITES", False) + monkeypatch.setattr(platform_util, "HANDLE_ANCHORED_WRITES", False) monkeypatch.setattr(platform_util, "DIR_FD_ANCHORED_WRITES", False) with pytest.raises(_Pause, match="attempt-owned lifecycle status"): @@ -1765,7 +1829,8 @@ def test_plain_tracked_snapshot_forced_fallback_pauses_before_write(project, mon flow = _make_flow( workspace=Workspace.default(project), policy=_policy(rollback_on_failure=False) ) - monkeypatch.setattr(recovery_flow, "DIR_FD_ANCHORED_WRITES", False) + monkeypatch.setattr(recovery_flow, "HANDLE_ANCHORED_WRITES", False) + monkeypatch.setattr(platform_util, "HANDLE_ANCHORED_WRITES", False) monkeypatch.setattr(platform_util, "DIR_FD_ANCHORED_WRITES", False) with pytest.raises(_Pause, match="manual adoption is required"): @@ -1831,7 +1896,8 @@ def test_plain_forced_fallback_pauses_after_completed_baseline_reset(project, mo flow = _make_flow( workspace=Workspace.default(project), policy=_policy(rollback_on_failure=True) ) - monkeypatch.setattr(recovery_flow, "DIR_FD_ANCHORED_WRITES", False) + monkeypatch.setattr(recovery_flow, "HANDLE_ANCHORED_WRITES", False) + monkeypatch.setattr(platform_util, "HANDLE_ANCHORED_WRITES", False) monkeypatch.setattr(platform_util, "DIR_FD_ANCHORED_WRITES", False) with pytest.raises(_Pause, match="after the baseline reset"): @@ -1906,7 +1972,8 @@ def test_plain_sibling_residue_forced_fallback_preempts_generic_manual_pause(pro flow = _make_flow( workspace=Workspace.default(project), policy=_policy(rollback_on_failure=False) ) - monkeypatch.setattr(recovery_flow, "DIR_FD_ANCHORED_WRITES", False) + monkeypatch.setattr(recovery_flow, "HANDLE_ANCHORED_WRITES", False) + monkeypatch.setattr(platform_util, "HANDLE_ANCHORED_WRITES", False) monkeypatch.setattr(platform_util, "DIR_FD_ANCHORED_WRITES", False) with pytest.raises(_Pause, match="before the ordinary manual-recovery pause"): @@ -1978,7 +2045,8 @@ def test_latched_redrive_snapshot_equal_forced_fallback_pauses_for_retry_input( flow = _make_flow( workspace=Workspace.default(project), policy=_policy(rollback_on_failure=False) ) - monkeypatch.setattr(recovery_flow, "DIR_FD_ANCHORED_WRITES", False) + monkeypatch.setattr(recovery_flow, "HANDLE_ANCHORED_WRITES", False) + monkeypatch.setattr(platform_util, "HANDLE_ANCHORED_WRITES", False) monkeypatch.setattr(platform_util, "DIR_FD_ANCHORED_WRITES", False) with pytest.raises(_Pause, match="pre-attempt retry input"): @@ -2050,7 +2118,8 @@ def test_latched_redrive_forced_fallback_pauses_after_preservation(project, monk flow = _make_flow( workspace=Workspace.default(project), policy=_policy(rollback_on_failure=False) ) - monkeypatch.setattr(recovery_flow, "DIR_FD_ANCHORED_WRITES", False) + monkeypatch.setattr(recovery_flow, "HANDLE_ANCHORED_WRITES", False) + monkeypatch.setattr(platform_util, "HANDLE_ANCHORED_WRITES", False) monkeypatch.setattr(platform_util, "DIR_FD_ANCHORED_WRITES", False) with pytest.raises(_Pause, match="manual adoption is required"): @@ -2088,7 +2157,8 @@ def test_latched_redrive_forced_fallback_pauses_after_completed_baseline_reset( flow = _make_flow( workspace=Workspace.default(project), policy=_policy(rollback_on_failure=True) ) - monkeypatch.setattr(recovery_flow, "DIR_FD_ANCHORED_WRITES", False) + monkeypatch.setattr(recovery_flow, "HANDLE_ANCHORED_WRITES", False) + monkeypatch.setattr(platform_util, "HANDLE_ANCHORED_WRITES", False) monkeypatch.setattr(platform_util, "DIR_FD_ANCHORED_WRITES", False) with pytest.raises(_Pause, match="after the baseline reset"): @@ -2210,7 +2280,8 @@ def test_plain_git_invisible_snapshot_forced_fallback_pauses_before_reset( flow = _make_flow( workspace=Workspace.default(project), policy=_policy(rollback_on_failure=True) ) - monkeypatch.setattr(recovery_flow, "DIR_FD_ANCHORED_WRITES", False) + monkeypatch.setattr(recovery_flow, "HANDLE_ANCHORED_WRITES", False) + monkeypatch.setattr(platform_util, "HANDLE_ANCHORED_WRITES", False) monkeypatch.setattr(platform_util, "DIR_FD_ANCHORED_WRITES", False) with pytest.raises(_Pause, match="manual adoption is required"): @@ -2297,7 +2368,8 @@ def test_latched_redrive_index_only_forced_fallback_pauses_after_preservation( flow = _make_flow( workspace=Workspace.default(project), policy=_policy(rollback_on_failure=False) ) - monkeypatch.setattr(recovery_flow, "DIR_FD_ANCHORED_WRITES", False) + monkeypatch.setattr(recovery_flow, "HANDLE_ANCHORED_WRITES", False) + monkeypatch.setattr(platform_util, "HANDLE_ANCHORED_WRITES", False) monkeypatch.setattr(platform_util, "DIR_FD_ANCHORED_WRITES", False) with pytest.raises(_Pause, match="before the baseline reset"): @@ -2573,7 +2645,8 @@ def test_resolved_cause_forced_fallback_pauses_after_completed_reset(project, mo flow = _make_flow( workspace=Workspace.default(project), policy=_policy(rollback_on_failure=False) ) - monkeypatch.setattr(recovery_flow, "DIR_FD_ANCHORED_WRITES", False) + monkeypatch.setattr(recovery_flow, "HANDLE_ANCHORED_WRITES", False) + monkeypatch.setattr(platform_util, "HANDLE_ANCHORED_WRITES", False) monkeypatch.setattr(platform_util, "DIR_FD_ANCHORED_WRITES", False) with pytest.raises(_Pause, match="after the baseline reset"): @@ -3085,7 +3158,8 @@ def test_patch_restore_redrive_forced_fallback_requires_in_review_adoption(proje flow = _make_flow( workspace=Workspace.default(project), policy=_policy(rollback_on_failure=False) ) - monkeypatch.setattr(recovery_flow, "DIR_FD_ANCHORED_WRITES", False) + monkeypatch.setattr(recovery_flow, "HANDLE_ANCHORED_WRITES", False) + monkeypatch.setattr(platform_util, "HANDLE_ANCHORED_WRITES", False) monkeypatch.setattr(platform_util, "DIR_FD_ANCHORED_WRITES", False) with pytest.raises(_Pause, match="lifecycle status 'in-review'"): diff --git a/tests/test_win32_at.py b/tests/test_win32_at.py new file mode 100644 index 00000000..8f9137a5 --- /dev/null +++ b/tests/test_win32_at.py @@ -0,0 +1,425 @@ +"""`bmad_loop.win32_at` — the handle-relative ``*at()`` family on Windows. + +Every row here but the last is Windows-only: the module builds its bindings +under ``sys.platform == "win32"`` alone, and there is no faking ``NtCreateFile``. +They run on the Windows CI legs, which is the only place the arm that replaced +the DW-309/DW-310 fail-closed pause is exercised for real; the recovery rows in +tests/test_recovery_flow.py ride on these primitives and pin the flow, not the +syscalls. Junctions stand in for directory symlinks throughout — ``mklink /J`` +needs no elevation, a directory symlink needs SeCreateSymbolicLinkPrivilege or +Developer Mode, and the junction is the redirect an unprivileged session can +actually plant, so it is the one the walk must refuse. +""" + +from __future__ import annotations + +import errno +import os +import stat +import sys +from contextlib import contextmanager +from pathlib import Path + +import pytest + +from bmad_loop import win32_at +from bmad_loop.platform_util import is_link_like + +WINDOWS = pytest.mark.skipif(sys.platform != "win32", reason="Windows handle-relative opens") + + +def _junction(link: Path, target: Path) -> None: + import _winapi # Windows-only stdlib module; CPython's own tests use it the same way + + _winapi.CreateJunction(str(target), str(link)) + + +@contextmanager +def _directory(path: Path): + fd = win32_at.open_directory(path) + try: + yield fd + finally: + os.close(fd) + + +# ---------------------------------------------------------------- open_directory + + +@WINDOWS +def test_open_directory_hands_back_a_descriptor_on_that_directory(tmp_path): + with _directory(tmp_path) as fd: + observed = os.fstat(fd) + assert stat.S_ISDIR(observed.st_mode) + assert os.path.samestat(observed, tmp_path.stat()) + + +@WINDOWS +def test_open_directory_refuses_a_file_and_a_missing_path(tmp_path): + (tmp_path / "file").write_bytes(b"x") + with pytest.raises(NotADirectoryError): + win32_at.open_directory(tmp_path / "file") + with pytest.raises(FileNotFoundError): + win32_at.open_directory(tmp_path / "absent") + + +@WINDOWS +def test_open_directory_follows_a_junction_unless_told_not_to(tmp_path): + """The root of a confined walk is opened WITH following, as the POSIX walk's + root is: the operator chooses where the project lives. ``follow=False`` is + the ``O_NOFOLLOW`` answer for the same entry, ``ELOOP``.""" + real = tmp_path / "real" + real.mkdir() + link = tmp_path / "link" + _junction(link, real) + + with _directory(link) as fd: + assert os.path.samestat(os.fstat(fd), real.stat()) + with pytest.raises(OSError) as refused: + win32_at.open_directory(link, follow=False) + assert refused.value.errno == errno.ELOOP + + +# ----------------------------------------------------------------------- open_at + + +@WINDOWS +def test_open_at_walks_components_relative_to_the_handle_above(tmp_path): + nested = tmp_path / "a" / "b" + nested.mkdir(parents=True) + with _directory(tmp_path) as root: + a = win32_at.open_at(root, "a", os.O_RDONLY | win32_at.AT_DIRECTORY | win32_at.AT_NOFOLLOW) + try: + b = win32_at.open_at(a, "b", os.O_RDONLY | win32_at.AT_DIRECTORY | win32_at.AT_NOFOLLOW) + finally: + os.close(a) + try: + assert os.path.samestat(os.fstat(b), nested.stat()) + finally: + os.close(b) + + +@WINDOWS +def test_open_at_refuses_anything_but_a_single_component(): + for name in ("", ".", "..", "a\\b", "a/b", "a\x00b"): + with pytest.raises(ValueError, match="single path component"): + win32_at.open_at(0, name, os.O_RDONLY) + + +@WINDOWS +def test_open_at_refuses_the_flags_it_cannot_honour(tmp_path): + """``O_TRUNC``/``O_APPEND`` have no handle-relative caller and no exact NT + spelling here; a loud refusal beats a silent approximation.""" + with _directory(tmp_path) as root: + for flag in (os.O_TRUNC, os.O_APPEND): + with pytest.raises(ValueError, match="O_TRUNC/O_APPEND"): + win32_at.open_at(root, "x", os.O_WRONLY | os.O_CREAT | flag) + assert list(tmp_path.iterdir()) == [] + + +@WINDOWS +def test_open_at_creates_exclusively_and_the_descriptor_is_an_ordinary_crt_fd(tmp_path): + """The whole point of wrapping the handle: the anchored writer keeps calling + ``os.write``/``os.fsync``/``os.fdopen``/``os.fstat``/``os.close`` on it.""" + with _directory(tmp_path) as root: + fd = win32_at.open_at(root, "spec.tmp", os.O_RDWR | os.O_CREAT | os.O_EXCL) + with os.fdopen(fd, "r+b") as fh: + fh.write(b"payload\r\n") + fh.flush() + os.fsync(fh.fileno()) + fh.seek(0) + assert fh.read() == b"payload\r\n" # binary: no newline translation + assert stat.S_ISREG(os.fstat(fh.fileno()).st_mode) + with pytest.raises(FileExistsError): + win32_at.open_at(root, "spec.tmp", os.O_RDWR | os.O_CREAT | os.O_EXCL) + with pytest.raises(FileNotFoundError): + win32_at.open_at(root, "absent", os.O_RDONLY) + assert (tmp_path / "spec.tmp").read_bytes() == b"payload\r\n" + + +@WINDOWS +def test_open_at_reads_a_directory_but_refuses_to_write_one(tmp_path): + """As ``open(2)``: a read-only open of a directory succeeds so the caller's + ``S_ISREG`` check sees it; a write open is ``EISDIR``.""" + (tmp_path / "dir").mkdir() + with _directory(tmp_path) as root: + fd = win32_at.open_at(root, "dir", os.O_RDONLY | win32_at.AT_NOFOLLOW) + try: + assert stat.S_ISDIR(os.fstat(fd).st_mode) + finally: + os.close(fd) + with pytest.raises(IsADirectoryError): + win32_at.open_at(root, "dir", os.O_WRONLY | win32_at.AT_NOFOLLOW) + + +@WINDOWS +def test_open_at_directory_flag_refuses_a_file(tmp_path): + (tmp_path / "file").write_bytes(b"x") + with _directory(tmp_path) as root: + with pytest.raises(NotADirectoryError): + win32_at.open_at(root, "file", os.O_RDONLY | win32_at.AT_DIRECTORY) + + +@WINDOWS +def test_open_at_nofollow_refuses_a_junction_with_eloop(tmp_path): + """The refusal the confined walk is built on: a junction planted at a + component below the root fails the walk exactly as ``O_NOFOLLOW`` fails a + symlink there — and nothing under the junction's target was touched.""" + outside = tmp_path / "outside" + outside.mkdir() + (tmp_path / "project").mkdir() + _junction(tmp_path / "project" / "artifacts", outside) + with _directory(tmp_path / "project") as project: + with pytest.raises(OSError) as refused: + win32_at.open_at( + project, "artifacts", os.O_RDONLY | win32_at.AT_DIRECTORY | win32_at.AT_NOFOLLOW + ) + assert refused.value.errno == errno.ELOOP + # Without the flag the junction is followed, as `os.open` would follow it. + fd = win32_at.open_at(project, "artifacts", os.O_RDONLY | win32_at.AT_DIRECTORY) + try: + assert os.path.samestat(os.fstat(fd), outside.stat()) + finally: + os.close(fd) + + +@WINDOWS +def test_open_at_write_probe_refuses_a_read_only_file(tmp_path): + """`_refuse_unwritable_target_at`'s contract: the operator's read-only + attribute answers ``PermissionError`` from the handle-relative open too.""" + target = tmp_path / "spec.md" + target.write_bytes(b"x") + target.chmod(stat.S_IREAD) + try: + with _directory(tmp_path) as root: + with pytest.raises(PermissionError): + win32_at.open_at(root, "spec.md", os.O_WRONLY | win32_at.AT_NOFOLLOW) + finally: + target.chmod(stat.S_IREAD | stat.S_IWRITE) + + +# ----------------------------------------------------------------------- stat_at + + +@WINDOWS +def test_stat_at_describes_the_entry_itself(tmp_path): + (tmp_path / "file").write_bytes(b"hello") + (tmp_path / "dir").mkdir() + with _directory(tmp_path) as root: + file_stat = win32_at.stat_at(root, "file") + assert stat.S_ISREG(file_stat.st_mode) + assert file_stat.st_size == 5 + assert os.path.samestat(file_stat, (tmp_path / "file").stat()) + assert stat.S_ISDIR(win32_at.stat_at(root, "dir").st_mode) + with pytest.raises(FileNotFoundError): + win32_at.stat_at(root, "absent") + + +@WINDOWS +def test_stat_at_reports_a_junction_as_a_link_not_a_directory(tmp_path): + """``lstat`` semantics: the recovery flow's ``S_ISREG``/``S_ISDIR`` checks must + answer False for a planted redirect, which ``fstat`` of the opened reparse + point would otherwise describe as the directory it stands in for.""" + real = tmp_path / "real" + real.mkdir() + _junction(tmp_path / "link", real) + with _directory(tmp_path) as root: + observed = win32_at.stat_at(root, "link") + assert stat.S_ISLNK(observed.st_mode) + assert not stat.S_ISDIR(observed.st_mode) and not stat.S_ISREG(observed.st_mode) + assert observed.st_reparse_tag == os.lstat(tmp_path / "link").st_reparse_tag + assert is_link_like(tmp_path / "link") + + +# -------------------------------------------------------------------- replace_at + + +@WINDOWS +def test_replace_at_publishes_the_staged_inode_over_the_target(tmp_path): + """The anchored writer's publication step: the temp is renamed over the + target while the writer's own handle stays open on it, and that handle now + IS the published file — the inode the recovery flow verifies afterwards.""" + target = tmp_path / "spec.md" + target.write_bytes(b"old") + with _directory(tmp_path) as root: + fd = win32_at.open_at(root, "spec.tmp", os.O_RDWR | os.O_CREAT | os.O_EXCL) + try: + os.write(fd, b"new") + staged = os.fstat(fd) + win32_at.replace_at(root, "spec.tmp", root, "spec.md") + assert os.path.samestat(os.fstat(fd), staged) + assert os.path.samestat(win32_at.stat_at(root, "spec.md"), staged) + finally: + os.close(fd) + assert sorted(p.name for p in tmp_path.iterdir()) == ["spec.md"] + assert target.read_bytes() == b"new" + + +@WINDOWS +def test_replace_at_moves_across_directories_relative_to_both_handles(tmp_path): + src_dir = tmp_path / "src" + dst_dir = tmp_path / "dst" + src_dir.mkdir() + dst_dir.mkdir() + (src_dir / "a").write_bytes(b"a") + (dst_dir / "b").write_bytes(b"b") + with _directory(src_dir) as src, _directory(dst_dir) as dst: + win32_at.replace_at(src, "a", dst, "b") + assert list(src_dir.iterdir()) == [] + assert (dst_dir / "b").read_bytes() == b"a" + + +@WINDOWS +def test_replace_at_missing_source_is_file_not_found(tmp_path): + with _directory(tmp_path) as root: + with pytest.raises(FileNotFoundError): + win32_at.replace_at(root, "absent", root, "spec.md") + + +@WINDOWS +def test_replace_at_replaces_a_target_another_share_delete_handle_holds_open(tmp_path): + """POSIX rename semantics (``FileRenameInformationEx``): a reader holding the + OLD target open keeps reading the old bytes, while the name already serves + the new file — what ``rename(2)`` does, and what the fallback class cannot.""" + target = tmp_path / "spec.md" + target.write_bytes(b"old") + with _directory(tmp_path) as root: + reader = win32_at.open_at(root, "spec.md", os.O_RDONLY) + try: + fd = win32_at.open_at(root, "spec.tmp", os.O_RDWR | os.O_CREAT | os.O_EXCL) + os.write(fd, b"new") + os.close(fd) + win32_at.replace_at(root, "spec.tmp", root, "spec.md") + assert os.read(reader, 10) == b"old" + finally: + os.close(reader) + assert target.read_bytes() == b"new" + + +@WINDOWS +def test_replace_at_is_a_sharing_violation_while_the_target_is_open_without_share_delete( + tmp_path, +): + """The retry `platform_util.replace_at` wraps this in exists for exactly this: + a handle opened the ordinary CRT way (no ``FILE_SHARE_DELETE``) blocks the + replace until it closes, as it blocks ``os.replace`` — and with the same + WinError 5/32 pair `_retry_on_sharing_violation` treats as transient.""" + target = tmp_path / "spec.md" + target.write_bytes(b"old") + (tmp_path / "spec.tmp").write_bytes(b"new") + with _directory(tmp_path) as root: + with target.open("rb"): + with pytest.raises(PermissionError) as refused: + win32_at.replace_at(root, "spec.tmp", root, "spec.md") + assert refused.value.winerror in (5, 32) # ACCESS_DENIED / SHARING_VIOLATION + win32_at.replace_at(root, "spec.tmp", root, "spec.md") # ...and clears with it + assert target.read_bytes() == b"new" + + +# --------------------------------------------------------------------- unlink_at + + +@WINDOWS +def test_unlink_at_removes_a_file_and_refuses_a_directory(tmp_path): + (tmp_path / "file").write_bytes(b"x") + (tmp_path / "dir").mkdir() + with _directory(tmp_path) as root: + win32_at.unlink_at(root, "file") + with pytest.raises(IsADirectoryError): + win32_at.unlink_at(root, "dir") + with pytest.raises(FileNotFoundError): + win32_at.unlink_at(root, "file") + assert sorted(p.name for p in tmp_path.iterdir()) == ["dir"] + + +@WINDOWS +def test_unlink_at_removes_the_name_while_the_writers_own_handle_is_open(tmp_path): + """The anchored writer's failure cleanup unlinks its temp while its own + handle is still open; with POSIX delete semantics the NAME is gone at once + (a fresh ``O_EXCL`` create of it succeeds) and the handle stays readable.""" + with _directory(tmp_path) as root: + fd = win32_at.open_at(root, "spec.tmp", os.O_RDWR | os.O_CREAT | os.O_EXCL) + try: + os.write(fd, b"staged") + win32_at.unlink_at(root, "spec.tmp") + assert not (tmp_path / "spec.tmp").exists() + again = win32_at.open_at(root, "spec.tmp", os.O_RDWR | os.O_CREAT | os.O_EXCL) + os.close(again) + os.lseek(fd, 0, os.SEEK_SET) + assert os.read(fd, 10) == b"staged" + finally: + os.close(fd) + + +# ----------------------------------------------- the anchor under a moved parent + + +@WINDOWS +def test_a_directory_with_a_handle_open_beneath_it_cannot_be_renamed(tmp_path): + """The Windows half of the parent-swap race the POSIX rows stage in + tests/test_recovery_flow.py (`posix_parent_swap_under_writer`): while the + anchored writer holds its temp open, the OS refuses to rename any directory + above it, so the swap cannot happen at all — the platform closes the race + before the descriptor has to. If this row ever reddens, those rows can run + here too.""" + parent = tmp_path / "artifacts" + parent.mkdir() + with _directory(parent) as anchor: + fd = win32_at.open_at(anchor, "spec.tmp", os.O_RDWR | os.O_CREAT | os.O_EXCL) + try: + with pytest.raises(PermissionError): + parent.rename(tmp_path / "moved") + finally: + os.close(fd) + assert parent.is_dir() and not (tmp_path / "moved").exists() + + +@WINDOWS +def test_handle_relative_operations_stay_with_a_renamed_directory(tmp_path): + """A directory whose only open handle is the anchor itself CAN be renamed + (the handle shares DELETE). Everything relative to that handle then lands in + the directory wherever it now lives — the property that makes the anchor an + anchor — and a junction planted at the old name is never consulted.""" + parent = tmp_path / "artifacts" + parent.mkdir() + (parent / "spec.md").write_bytes(b"old") + outside = tmp_path / "outside" + outside.mkdir() + moved = tmp_path / "moved" + with _directory(parent) as anchor: + parent.rename(moved) + _junction(parent, outside) + assert os.path.samestat(win32_at.stat_at(anchor, "spec.md"), (moved / "spec.md").stat()) + fd = win32_at.open_at(anchor, "spec.tmp", os.O_RDWR | os.O_CREAT | os.O_EXCL) + try: + os.write(fd, b"new") + win32_at.replace_at(anchor, "spec.tmp", anchor, "spec.md") + finally: + os.close(fd) + assert (moved / "spec.md").read_bytes() == b"new" + assert list(outside.iterdir()) == [] + + +# --------------------------------------------------------------- off-Windows arm + + +@pytest.mark.skipif(sys.platform == "win32", reason="the stub arm") +def test_off_windows_every_call_is_enosys(): + """Importable everywhere, usable nowhere but Windows: the POSIX callers go + through ``os`` with ``dir_fd`` and never reach these.""" + assert not win32_at.AVAILABLE + calls = [ + lambda: win32_at.open_directory(Path(".")), + lambda: win32_at.open_at(0, "x", os.O_RDONLY), + lambda: win32_at.stat_at(0, "x"), + lambda: win32_at.replace_at(0, "x", 0, "y"), + lambda: win32_at.unlink_at(0, "x"), + ] + for call in calls: + with pytest.raises(OSError) as refused: + call() + assert refused.value.errno == errno.ENOSYS + # The flag vocabulary IS the os.O_* one on POSIX, so a caller composing + # `os.O_RDONLY | AT_NOFOLLOW` hands `os.open(dir_fd=...)` the real bit. + assert win32_at.AT_NOFOLLOW == os.O_NOFOLLOW + assert win32_at.AT_DIRECTORY == os.O_DIRECTORY From 6da88e516135ae0f1c495f547284185c3774e4c9 Mon Sep 17 00:00:00 2001 From: t Date: Wed, 16 Sep 2026 17:36:44 -0700 Subject: [PATCH 2/5] docs(changelog): wave5 S13 entries --- CHANGELOG.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9308a563..7192b609 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,16 @@ breaking changes may land in a minor release. ### Added +- Restore and normalize attempt-owned specs on Windows through NT handle-relative + opens instead of pausing for manual adoption. `win32_at` binds `NtCreateFile` with a + `RootDirectory` handle plus `FileRenameInformationEx`/`FileDispositionInformationEx` + (POSIX semantics, classic classes as the fallback) and refuses a symlink or junction + as `O_NOFOLLOW` refuses a link; `platform_util.HANDLE_ANCHORED_WRITES` joins that arm + to the POSIX `dir_fd` one behind `open_at`/`stat_at`/`replace_at`/`unlink_at`, so the + confined spec writers and `open_dir_confined` anchor at a handle on both hosts. The + DW-309/DW-310 refusal now fires only on a host with neither arm; its journaled + `problem` reads `lacks handle-anchored writes`. + - Accept a session-asserted artifact-only sweep bundle at the dev proof-of-work gate (DW-273). A bundle whose only deliverable lives under a gitignored `implementation_artifacts` (a spec-only erratum) burned every attempt on From c1403e1ac4e4b670465c650621b36fcce9bb6e72 Mon Sep 17 00:00:00 2001 From: t Date: Wed, 16 Sep 2026 18:10:23 -0700 Subject: [PATCH 3/5] fix(win32_at): request FILE_READ_ATTRIBUTES on every handle-relative open MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Windows CI (#799, both legs): 283 rows failed with PermissionError [WinError 5] out of _refuse_unwritable_target_at → open_at(O_WRONLY | AT_NOFOLLOW) → _refuse_link. FILE_GENERIC_WRITE carries WRITE_ATTRIBUTES only, so the write-only open succeeded and GetFileInformationByHandleEx (FileAttributeTagInfo) was then denied on that handle. CreateFileW adds FILE_READ_ATTRIBUTES to every open implicitly; do the same here. Wine granted the read regardless, which is why the harness never showed it. --- src/bmad_loop/win32_at.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/bmad_loop/win32_at.py b/src/bmad_loop/win32_at.py index 00eb7ecc..90d37508 100644 --- a/src/bmad_loop/win32_at.py +++ b/src/bmad_loop/win32_at.py @@ -359,7 +359,11 @@ def open_at(dir_fd: int, name: str, flags: int, mode: int = 0o600) -> int: if flags & (os.O_TRUNC | os.O_APPEND): raise ValueError("O_TRUNC/O_APPEND are not supported relative to a handle") writable = accmode in (os.O_WRONLY, os.O_RDWR) - access = 0 + # FILE_READ_ATTRIBUTES on every open, as CreateFileW grants implicitly: + # `_refuse_link`'s GetFileInformationByHandleEx needs it, and a write-only + # open (FILE_GENERIC_WRITE carries only WRITE_ATTRIBUTES) is otherwise + # answered ERROR_ACCESS_DENIED at the attribute read, not at the open. + access = _FILE_READ_ATTRIBUTES if accmode in (os.O_RDONLY, os.O_RDWR): access |= _FILE_GENERIC_READ if writable: From 913f3c021b683b370251fbbf2255c03f046ccd19 Mon Sep 17 00:00:00 2001 From: t Date: Wed, 16 Sep 2026 18:16:36 -0700 Subject: [PATCH 4/5] test(conftest): patch the publish rename on the Windows anchored arm too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three rows inject a fault at `os.replace` (the floor every POSIX publish reaches, path-based or dir_fd-relative): the two no-tmp-residue rows in test_operatoractions and the interleaved-writer row in test_runs. With the Windows confined writer now handle-anchored, its publish is `win32_at.replace_at` (NtSetInformationFile), so that patch fired on nothing there — DID NOT RAISE / `[] == ['b']` on both Windows legs of #799. `conftest.patch_publish_rename` patches `os.replace` and, on the arm where it is live, `win32_at.replace_at` with the argument order adapted; `real_publish_rename` is the matching fall-through, bound to the originals at import. Linux behaviour is unchanged (same `os.replace` patch). Ablations: dropping either arm's patch reddens the rows on that arm (Linux; Wine). --- tests/conftest.py | 44 ++++++++++++++++++++++++++++++++++- tests/test_operatoractions.py | 25 +++++++++++++------- tests/test_runs.py | 17 ++++++++++---- 3 files changed, 73 insertions(+), 13 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index d64dacf9..2f92089b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -20,7 +20,7 @@ import pytest import yaml -from bmad_loop import cli, documents, envvars, platform_util, runs +from bmad_loop import cli, documents, envvars, platform_util, runs, win32_at from bmad_loop.adapters.base import SessionResult, SessionSpec from bmad_loop.bmadconfig import ProjectPaths, load_paths from bmad_loop.checks import ValidationReport @@ -595,6 +595,48 @@ def remove_tree(path: Path) -> None: shutil.rmtree(path) +_REAL_OS_REPLACE = os.replace +_REAL_WIN32_REPLACE_AT = win32_at.replace_at + + +def real_publish_rename(src, dst, *, src_dir_fd=None, dst_dir_fd=None) -> None: + """The rename a :func:`patch_publish_rename` interceptor falls through to. + + ``os.replace``'s shape, routed to the syscall the arm actually publishes with: + a path-based rename is ``os.replace``, a dir_fd-relative one is ``os.replace`` + on POSIX and ``win32_at.replace_at`` on Windows (where ``os.replace`` refuses + ``dir_fd`` outright). Bound to the ORIGINALS at import, so calling it from + inside the interceptor never re-enters the patch.""" + if src_dir_fd is None and dst_dir_fd is None: + _REAL_OS_REPLACE(src, dst) + elif platform_util.DIR_FD_ANCHORED_WRITES: + _REAL_OS_REPLACE(src, dst, src_dir_fd=src_dir_fd, dst_dir_fd=dst_dir_fd) + else: + _REAL_WIN32_REPLACE_AT(src_dir_fd, src, dst_dir_fd, dst) + + +def patch_publish_rename(monkeypatch: pytest.MonkeyPatch, fn) -> None: + """Route every rename a writer publishes with through ``fn``, on either arm. + + ``fn(src, dst, *, src_dir_fd=None, dst_dir_fd=None)`` — ``os.replace``'s shape, + because on POSIX that IS the floor every publish reaches: the path-based + ``atomic_replace`` and the confined writer's anchored arm (a bare + dir_fd-relative ``os.replace``, #593) alike, so one patch covers both the + row's ablation and a reversion to the hand-rolled ``tmp + atomic_replace``. + Windows' anchored arm renames through ``win32_at.replace_at`` (handle-relative + ``NtSetInformationFile``; ``os.replace`` is never called), so a test patching + only ``os.replace`` fires on nothing there and passes having faulted nobody. + That name is patched too, with the argument order adapted, on the arm where + it is live. ``fn`` falls through with :func:`real_publish_rename`.""" + monkeypatch.setattr(os, "replace", fn) + if win32_at.AVAILABLE: + + def _at(src_dir_fd: int, src: str, dst_dir_fd: int, dst: str) -> None: + fn(src, dst, src_dir_fd=src_dir_fd, dst_dir_fd=dst_dir_fd) + + monkeypatch.setattr(win32_at, "replace_at", _at) + + NOISY_GIT_KEY = "core.fsyncMethod" NOISY_GIT_VALUE = "bmad-loop-not-a-method" diff --git a/tests/test_operatoractions.py b/tests/test_operatoractions.py index 84a0fa43..c8085639 100644 --- a/tests/test_operatoractions.py +++ b/tests/test_operatoractions.py @@ -10,11 +10,18 @@ from __future__ import annotations import json -import os import sys import pytest -from conftest import git, install_bmad_config, spec_path, write_spec, write_sprint +from conftest import ( + git, + install_bmad_config, + patch_publish_rename, + real_publish_rename, + spec_path, + write_spec, + write_sprint, +) from bmad_loop import devcontract, operatoractions, platform_util, verify @@ -178,19 +185,21 @@ def test_a_failed_record_write_leaves_no_tmp_residue(project, monkeypatch): `os.replace`, not `platform_util.atomic_replace`: the confined writer's anchored arm publishes with a bare dir_fd-relative `os.replace` and never reaches `atomic_replace` at all, so the old injection point would no longer - fire and this test would pass having faulted nothing. Filtered to the record's - own name so an unrelated replace during the test is not collateral. + fire and this test would pass having faulted nothing. Through + `patch_publish_rename`, which also covers the Windows anchored arm's + `win32_at.replace_at` for the same reason. Filtered to the record's own name + so an unrelated replace during the test is not collateral. Ablation: drop the `os.unlink(tmp, dir_fd=dir_fd)` in `platform_util._atomic_write_at`'s except arm and this fails.""" - real_replace = os.replace + real_replace = real_publish_rename def boom(src, dst, *, src_dir_fd=None, dst_dir_fd=None): if str(dst).endswith(".json"): raise OSError(28, "No space left on device") return real_replace(src, dst, src_dir_fd=src_dir_fd, dst_dir_fd=dst_dir_fd) - monkeypatch.setattr(os, "replace", boom) + patch_publish_rename(monkeypatch, boom) with pytest.raises(OSError): operatoractions.record_park( project.project, @@ -332,7 +341,7 @@ def test_a_failed_legacy_prune_leaves_no_tmp_residue(project, monkeypatch): Ablation: drop the `os.unlink(tmp, dir_fd=dir_fd)` in `platform_util._atomic_write_at`'s except arm and this fails.""" - real_replace = os.replace + real_replace = real_publish_rename def boom(src, dst, *, src_dir_fd=None, dst_dir_fd=None): if str(dst).endswith(".json"): @@ -345,7 +354,7 @@ def boom(src, dst, *, src_dir_fd=None, dst_dir_fd=None): before = sorted(p.name for p in store.parent.iterdir()) assert before == ["operator-actions.json"] # a snapshot with something IN it - monkeypatch.setattr(os, "replace", boom) + patch_publish_rename(monkeypatch, boom) with pytest.raises(OSError): operatoractions.drop(project.project, "1-1-a") diff --git a/tests/test_runs.py b/tests/test_runs.py index 1f102720..369a4901 100644 --- a/tests/test_runs.py +++ b/tests/test_runs.py @@ -15,7 +15,14 @@ from unittest import mock import pytest -from conftest import assert_run_state_lock_held, escalated_run, git, refuse_to_resolve +from conftest import ( + assert_run_state_lock_held, + escalated_run, + git, + patch_publish_rename, + real_publish_rename, + refuse_to_resolve, +) from bmad_loop import envvars, platform_util, runs, verify from bmad_loop.adapters import tmux_base @@ -1290,12 +1297,14 @@ def test_write_stop_request_survives_an_interleaved_concurrent_writer(tmp_path, seam still covers the ablation the old dual patch existed for, and covers it better — `atomic_replace` is itself a wrapper around `os.replace`, so reverting `_write_stop_request` to the hand-rolled `tmp + atomic_replace` - routes through this same patch and must still redden this test. + routes through this same patch and must still redden this test. Through + `patch_publish_rename`, which also covers the Windows anchored arm's + `win32_at.replace_at` — there `os.replace` is never called at all. Filtered to the stop-request name so an unrelated replace during the test is not collateral.""" run_dir = _make_state_run(tmp_path, "r1") - real_replace = os.replace + real_replace = real_publish_rename nested: list[str] = [] def _interleave(src, dst, *, src_dir_fd=None, dst_dir_fd=None): @@ -1304,7 +1313,7 @@ def _interleave(src, dst, *, src_dir_fd=None, dst_dir_fd=None): runs._write_stop_request(run_dir, "graceful") return real_replace(src, dst, src_dir_fd=src_dir_fd, dst_dir_fd=dst_dir_fd) - monkeypatch.setattr(os, "replace", _interleave) + patch_publish_rename(monkeypatch, _interleave) runs._write_stop_request(run_dir, "hard") # writer A — must not raise From 1c1759694945aa294502bb162dde9b604b6d4b7c Mon Sep 17 00:00:00 2001 From: t Date: Sun, 20 Sep 2026 10:40:31 -0700 Subject: [PATCH 5/5] fix(win32_at): size NT names in UTF-16 code units and the rename record by pointer width MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_nt_open` measured `UNICODE_STRING.Length` as `len(name) * 2` — code points, not the UTF-16 code units `create_unicode_buffer` already sizes the buffer in — so a non-BMP character in a component (an emoji in a spec name) handed NtCreateFile a name one WCHAR short: the read was FileNotFoundError for a file that exists, and an exclusive create planted a truncated entry beside it. Length now comes from the buffer itself. `_rename_information` packed the 64-bit FILE_RENAME_INFORMATION layout unconditionally; on a 32-bit interpreter HANDLE is 4 bytes and the kernel read the padding as RootDirectory and half the handle as FileNameLength, failing every replace_at. The header format now follows the pointer width, and the name is encoded with surrogatepass as the open buffer already tolerates. Both verified under Wine on the 3.12 amd64 and win32 embeddable builds, with each fix ablated to confirm its test fails. --- src/bmad_loop/win32_at.py | 23 ++++++++++++---- tests/test_win32_at.py | 58 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 6 deletions(-) diff --git a/src/bmad_loop/win32_at.py b/src/bmad_loop/win32_at.py index 90d37508..13da3730 100644 --- a/src/bmad_loop/win32_at.py +++ b/src/bmad_loop/win32_at.py @@ -267,8 +267,12 @@ def _nt_open( attributes: int = _FILE_ATTRIBUTE_NORMAL, ) -> int: buf = ctypes.create_unicode_buffer(name) - length = len(name) * 2 - unicode = _UNICODE_STRING(length, length + 2, ctypes.cast(buf, wt.LPWSTR)) + # UNICODE_STRING counts bytes of UTF-16 code units, not code points: a + # non-BMP character (an emoji in a spec name) is a surrogate pair, two + # WCHARs. The buffer is sized that way already; `len(name) * 2` is not. + maximum = ctypes.sizeof(buf) + length = maximum - ctypes.sizeof(ctypes.c_wchar) + unicode = _UNICODE_STRING(length, maximum, ctypes.cast(buf, wt.LPWSTR)) attrs = _OBJECT_ATTRIBUTES( ctypes.sizeof(_OBJECT_ATTRIBUTES), root_handle, @@ -435,13 +439,20 @@ def stat_at(dir_fd: int, name: str) -> os.stat_result: }, ) + # FILE_RENAME_INFORMATION: union{BOOLEAN ReplaceIfExists; ULONG Flags}; + # HANDLE RootDirectory; ULONG FileNameLength; WCHAR FileName[]. The HANDLE + # is pointer-aligned, so the layout follows the interpreter's pointer width: + # 64-bit pads the union to 8 and puts the name at offset 20; 32-bit packs + # the three fields and puts it at 12. + _RENAME_HEADER = " bytes: - # FILE_RENAME_INFORMATION: union{BOOLEAN ReplaceIfExists; ULONG Flags}; - # ; HANDLE RootDirectory; ULONG FileNameLength; WCHAR FileName[]. # A ULONG 1 in the union reads as ReplaceIfExists=TRUE for the classic # class and as REPLACE_IF_EXISTS for the Ex one, so one buffer serves both. - encoded = name.encode("utf-16-le") - return struct.pack(" int: buf = ctypes.create_string_buffer(payload, len(payload)) diff --git a/tests/test_win32_at.py b/tests/test_win32_at.py index 8f9137a5..f6e35992 100644 --- a/tests/test_win32_at.py +++ b/tests/test_win32_at.py @@ -99,6 +99,35 @@ def test_open_at_walks_components_relative_to_the_handle_above(tmp_path): os.close(b) +@WINDOWS +def test_open_at_measures_the_name_in_utf16_code_units(tmp_path): + """A non-BMP character (an emoji in a spec or artifacts name) is a surrogate + pair — two WCHARs — so ``UNICODE_STRING.Length`` counts more bytes than + ``len(name) * 2``. Ablation: measure by code points and every open, stat and + exclusive create of such a name lands on a name one WCHAR short: the read + is ``FileNotFoundError`` for a file that exists, and the create plants a + truncated entry beside it.""" + name = "spec-\U0001f680.md" # U+1F680 ROCKET, outside the BMP + assert len(name.encode("utf-16-le")) == len(name) * 2 + 2 + (tmp_path / name).write_bytes(b"payload") + with _directory(tmp_path) as root: + fd = win32_at.open_at(root, name, os.O_RDONLY | win32_at.AT_NOFOLLOW) + try: + assert os.read(fd, 16) == b"payload" + assert os.path.samestat(os.fstat(fd), (tmp_path / name).stat()) + finally: + os.close(fd) + assert os.path.samestat(win32_at.stat_at(root, name), (tmp_path / name).stat()) + with pytest.raises(FileExistsError): + win32_at.open_at(root, name, os.O_RDWR | os.O_CREAT | os.O_EXCL) + staged = "staged-\U0001f680.tmp" + os.close(win32_at.open_at(root, staged, os.O_RDWR | os.O_CREAT | os.O_EXCL)) + assert sorted(p.name for p in tmp_path.iterdir()) == sorted([name, staged]) + win32_at.replace_at(root, staged, root, name) + win32_at.unlink_at(root, name) + assert list(tmp_path.iterdir()) == [] + + @WINDOWS def test_open_at_refuses_anything_but_a_single_component(): for name in ("", ".", "..", "a\\b", "a/b", "a\x00b"): @@ -269,6 +298,35 @@ def test_replace_at_moves_across_directories_relative_to_both_handles(tmp_path): assert (dst_dir / "b").read_bytes() == b"a" +@WINDOWS +def test_rename_information_follows_the_native_pointer_width(): + """``FILE_RENAME_INFORMATION`` places ``RootDirectory`` at pointer alignment, + so the name begins at offset 20 on a 64-bit interpreter and 12 on a 32-bit + one. The layout is held against ctypes' own native alignment of the header, + not a second copy of the arithmetic. Ablation: pack the 64-bit shape + unconditionally and a 32-bit Python hands the kernel its padding as + ``RootDirectory`` — every ``replace_at`` fails.""" + import ctypes + import ctypes.wintypes as wt + + class Header(ctypes.Structure): + _fields_ = ( + ("Flags", wt.ULONG), + ("RootDirectory", wt.HANDLE), + ("FileNameLength", wt.ULONG), + ) + + name_offset = Header.FileNameLength.offset + ctypes.sizeof(wt.ULONG) + encoded = "spec-\U0001f680.md".encode("utf-16-le") + payload = win32_at._rename_information(0x3, 0x1234, "spec-\U0001f680.md") + header = Header.from_buffer_copy(payload[: ctypes.sizeof(Header)]) + assert header.Flags == 0x3 + assert header.RootDirectory == 0x1234 + assert header.FileNameLength == len(encoded) + assert payload[name_offset : name_offset + len(encoded)] == encoded + assert payload[name_offset + len(encoded) :] == b"\0\0" + + @WINDOWS def test_replace_at_missing_source_is_file_not_found(tmp_path): with _directory(tmp_path) as root: