diff --git a/docs/adr/0005-fenced-windows-setup-owner.md b/docs/adr/0005-fenced-windows-setup-owner.md index 4754a41..e2373a8 100644 --- a/docs/adr/0005-fenced-windows-setup-owner.md +++ b/docs/adr/0005-fenced-windows-setup-owner.md @@ -33,26 +33,47 @@ Each attempt has three create-once JSON records under `setup-attempts`: - `launch-receipt.json` records the detached keeper and suspended root process identities before resume. - `terminal.json` records process outcome, bounded output, and whether cleanup is proven. -The detached keeper creates a kill-on-close Job Object, then creates system -PowerShell suspended with `PROC_THREAD_ATTRIBUTE_JOB_LIST`. The child inherits -only its three standard-stream pipe handles. The keeper durably publishes the -launch receipt before it resumes the root process. It sends the already -validated script bytes through standard input to a fixed bootstrap. That -bootstrap checks the exact byte count and SHA-256, decodes strict UTF-8 in -memory, and only then invokes the script block. +The detached keeper creates an unnamed kill-on-close Job Object, then creates +system PowerShell suspended with `PROC_THREAD_ATTRIBUTE_JOB_LIST`. Because the +Job has no name and its handle is not inherited, the child cannot reopen or +retain it. The child inherits only its three standard-stream pipe handles. The +keeper durably publishes the launch receipt before it resumes the root process. +It sends the already validated script bytes through standard input to a fixed +bootstrap. That bootstrap checks the exact byte count and SHA-256, decodes +strict UTF-8 in memory, and only then invokes the script block. + +Every state, attempt, record, marker, and script access opens the complete +Windows path chain without delete sharing and with reparse-point traversal +disabled. The owner rejects a reparse point in any ancestor. The authority root +must exist with DACL inheritance disabled. The owner also rejects a target with +a null DACL, an untrusted owner, or write-capable allow entries—including +inherit-only entries—for principals other than the interactive user, Local +System, built-in Administrators, and Creator Owner. Reserved compound allow +entries fail closed instead of being ignored. The handles remain open for the +protected operation so an ancestor cannot be renamed underneath it. Status and stop require the complete Attempt, request-hash, and Launch fence. Normal stop asks the exact keeper to terminate its Job. Fallback termination opens the recorded keeper once and verifies its creation FILETIME through that -handle. PID lookup alone never authorizes termination. Owner-loss recovery -claims `tree_gone` only after the exact keeper and root are absent and the -named Job no longer exists. Otherwise the immutable terminal is +handle. PID lookup alone never authorizes termination. If a new unnamed-Job +keeper disappears before publishing a terminal, recovery cannot prove that no +same-token process duplicated its handle, so it records `cleanup_unverified` +even when the recorded keeper and root are gone. Access or query failure is +also unknown, not absence. Status reports the nonterminal, +`ownership_unverified` state so the caller can retry. Stop still attempts the +exact identity-fenced keeper termination, but reports the same retryable state +if process access or termination fails; it does not make that transient failure +immutable. Receipts from the earlier named-Job implementation remain readable +during upgrade, but recovery never treats absence of their session-local Job +name as proof that the tree is gone. A live legacy keeper may still publish its +own terminal after stop; owner-loss or fallback recovery reports `cleanup_unverified`. `windows-setup-owner-v1` is a runtime capability. Its probe creates an inert suspended child with the same atomic Job-list mechanism, verifies membership, -terminates the Job before resume, and proves that it has no members. Missing -API support or an incompatible outer Job policy fails closed. +closes the keeper's sole Job handle before resume, and proves kill-on-close by +observing the root process exit. Missing API support or an incompatible outer +Job policy fails closed. ## Consequences @@ -61,6 +82,8 @@ without broad process discovery. A stale caller cannot act on a replacement attempt. The keeper is the only process that retains the Job handle, so its loss terminates the setup tree. -The protocol is Windows-only and private to Blender Box setup. Staging and its -ACL handoff remain Blender Box responsibilities. The owner rejects a staged -artifact that does not match the immutable request. +The protocol is Windows-only and private to Blender Box setup. Blender Box must +stage into an authority root with an explicit private DACL; inherited temporary +or profile-directory ACLs are not assumed safe. The owner independently +validates that path authority and rejects an artifact that does not match the +immutable request. diff --git a/src/blendersessiond/cli.py b/src/blendersessiond/cli.py index f8172dc..d7883b5 100644 --- a/src/blendersessiond/cli.py +++ b/src/blendersessiond/cli.py @@ -379,7 +379,14 @@ def main(argv: Sequence[str] | None = None) -> int: ) return 1 _print_result(result.to_dict(), as_json=args.json) - return 0 + if getattr(result, "ownership_unverified", False): + return 1 + terminal = getattr(result, "terminal", None) + if terminal is None: + return 0 + if args.setup_owner_command == "stop": + return 0 if terminal.cleanup == "tree_gone" else 1 + return 0 if terminal.succeeded else 1 raise AssertionError(f"unhandled command: {args.command}") diff --git a/src/blendersessiond/setup_owner.py b/src/blendersessiond/setup_owner.py index 331b38d..9a3ef70 100644 --- a/src/blendersessiond/setup_owner.py +++ b/src/blendersessiond/setup_owner.py @@ -10,7 +10,8 @@ import subprocess import sys import time -from collections.abc import Callable +from collections.abc import Callable, Iterator +from contextlib import contextmanager from dataclasses import dataclass from datetime import UTC, datetime, timedelta from pathlib import Path @@ -33,6 +34,7 @@ _ATTEMPT_ID = re.compile(r"^bbsa_[A-Za-z0-9_-]{43}$") _LAUNCH_ID = re.compile(r"^bbsl_[A-Za-z0-9_-]{43}$") _SHA256 = re.compile(r"^[0-9a-f]{64}$") +JOB_SCOPE = "unnamed-kill-on-close" _TERMINAL_OUTCOMES = { "cancelled", "cleanup_unverified", @@ -109,11 +111,12 @@ class LaunchReceipt: keeper_creation_time: str root_pid: int root_creation_time: str - job_name: str + job_scope: str | None owned_at: datetime + job_name: str | None = None def to_dict(self) -> dict[str, Any]: - return { + payload = { "schema_version": SCHEMA_VERSION, "attempt_id": self.attempt_id, "launch_id": self.launch_id, @@ -122,9 +125,13 @@ def to_dict(self) -> dict[str, Any]: "keeper_creation_time": self.keeper_creation_time, "root_pid": self.root_pid, "root_creation_time": self.root_creation_time, - "job_name": self.job_name, "owned_at": _format_time(self.owned_at), } + if self.job_name is not None: + payload["job_name"] = self.job_name + else: + payload["job_scope"] = self.job_scope + return payload @dataclass(frozen=True) @@ -213,6 +220,7 @@ class SetupView: request: SetupRequest receipt: LaunchReceipt | None = None terminal: SetupTerminal | None = None + ownership_unverified: bool = False def to_dict(self) -> dict[str, Any]: if self.terminal is not None: @@ -222,7 +230,13 @@ def to_dict(self) -> dict[str, Any]: "attempt_id": self.request.attempt_id, "launch_id": self.request.launch_id, "request_sha256": self.request.request_sha256, - "status": "owned" if self.receipt is not None else "accepted", + "status": ( + "ownership_unverified" + if self.ownership_unverified + else "owned" + if self.receipt is not None + else "accepted" + ), } if self.receipt is not None: payload["receipt"] = self.receipt.to_dict() @@ -238,8 +252,9 @@ def accept_launch_request( current = datetime.now(UTC) if now is None else now request = _parse_request(raw_request) directory = _attempt_directory(state_root, request.attempt_id) - directory.mkdir(mode=0o700, parents=True, exist_ok=True) - with file_lock(directory / ".lock"): + with _guard_setup_path(directory, allow_missing=True): + directory.mkdir(mode=0o700, parents=True, exist_ok=True) + with _setup_lock(directory): existing = _read_optional_record(directory / "request.json") if existing is None: if ( @@ -270,23 +285,44 @@ def launch_setup( request = accept_launch_request(raw_request, state_root=root, now=now) directory = _attempt_directory(root, request.attempt_id) should_dispatch = False - with file_lock(directory / ".lock"): + with _setup_lock(directory): view = _read_view(directory, request) if view.terminal is not None or view.receipt is not None: return view should_dispatch = _touch_create_once(directory / ".dispatch") if should_dispatch: - if dispatch_keeper is not None: - dispatch_keeper(directory) - return SetupView(request=request) - _spawn_keeper(directory) + try: + if dispatch_keeper is not None: + dispatch_keeper(directory) + return SetupView(request=request) + _spawn_keeper(directory) + except (OSError, RuntimeError, ValueError) as error: + with _setup_lock(directory): + view = _read_view(directory, request) + if view.receipt is not None or view.terminal is not None: + return view + _touch_create_once(directory / ".stop") + terminal = SetupTerminal.owned( + request=request, + outcome="launch_failed", + process="not_started", + cleanup="cleanup_unverified", + stdout=b"", + stderr=b"", + finished_at=datetime.now(UTC), + message=f"{type(error).__name__}: {error}"[:4096], + ) + _write_create_once( + directory / "terminal.json", terminal.to_dict() + ) + return SetupView(request=request, terminal=terminal) deadline = time.monotonic() + _LAUNCH_WAIT_SECONDS while time.monotonic() < deadline: view = _read_view(directory, request) if view.receipt is not None or view.terminal is not None: return view time.sleep(0.02) - with file_lock(directory / ".lock"): + with _setup_lock(directory): view = _read_view(directory, request) if view.receipt is not None or view.terminal is not None: return view @@ -308,8 +344,8 @@ def launch_setup( def status_setup( attempt_id: str, *, - expected_request_sha256: str | None = None, - expected_launch_id: str | None = None, + expected_request_sha256: str, + expected_launch_id: str, state_root: Path | None = None, platform_name: str | None = None, ) -> SetupView: @@ -323,11 +359,11 @@ def status_setup( return view if view.receipt is None: if ( - not (directory / ".dispatch").exists() + not _marker_exists(directory / ".dispatch") or request.deadline_utc > datetime.now(UTC) ): return view - with file_lock(directory / ".lock"): + with _setup_lock(directory): view = _read_view(directory, request) if view.terminal is not None or view.receipt is not None: return view @@ -345,8 +381,13 @@ def status_setup( _write_create_once(directory / "terminal.json", terminal.to_dict()) return SetupView(request=request, terminal=terminal) receipt = view.receipt - if process_matches(receipt.keeper_pid, receipt.keeper_creation_time): + keeper_state = _process_state( + receipt.keeper_pid, receipt.keeper_creation_time + ) + if keeper_state == "matches": return view + if keeper_state == "unknown": + return _unverified_view(request, receipt) return _reconcile_owner_loss(directory, request, receipt) @@ -364,11 +405,11 @@ def stop_setup( directory = _attempt_directory(root, attempt_id) request = _load_request(directory) _check_fence(request, expected_request_sha256, expected_launch_id) - with file_lock(directory / ".lock"): + with _setup_lock(directory): view = _read_view(directory, request) if view.terminal is not None: return view - if view.receipt is None and not (directory / ".dispatch").exists(): + if view.receipt is None and not _marker_exists(directory / ".dispatch"): terminal = SetupTerminal.owned( request=request, outcome="stopped_before_ownership", @@ -413,28 +454,21 @@ def stop_setup( current = _read_view(directory, request) if current.terminal is not None: return current - if not process_matches(receipt.keeper_pid, receipt.keeper_creation_time): + if _process_state( + receipt.keeper_pid, receipt.keeper_creation_time + ) != "matches": break time.sleep(0.05) - if process_matches(receipt.keeper_pid, receipt.keeper_creation_time): + keeper_state = _process_state( + receipt.keeper_pid, receipt.keeper_creation_time + ) + if keeper_state in {"matches", "unknown"}: from blendersessiond.windows_setup_process import terminate_exact_process try: terminate_exact_process(receipt.keeper_pid, receipt.keeper_creation_time) except (OSError, RuntimeError): - return _publish_terminal( - directory, - request, - SetupTerminal.owned( - request=request, - outcome="cleanup_unverified", - process="cancelled", - cleanup="cleanup_unverified", - stdout=b"", - stderr=b"", - finished_at=datetime.now(UTC), - ), - ) + return _unverified_view(request, receipt) return _reconcile_after_stop(directory, request, receipt) @@ -445,22 +479,29 @@ def script_path_for_attempt(state_root: Path, attempt_id: str) -> Path: def read_staged_script(state_root: Path, request: SetupRequest) -> bytes: path = script_path_for_attempt(state_root, request.attempt_id) try: - path_metadata = path.lstat() - if stat.S_ISLNK(path_metadata.st_mode) or not stat.S_ISREG( - path_metadata.st_mode - ): - raise SetupOwnerError("The staged setup script must be a regular file.") - with path.open("rb") as stream: - opened_metadata = os.fstat(stream.fileno()) - if ( - not stat.S_ISREG(opened_metadata.st_mode) - or (path_metadata.st_dev, path_metadata.st_ino) - != (opened_metadata.st_dev, opened_metadata.st_ino) + with _guard_setup_path(path): + path_metadata = path.lstat() + if stat.S_ISLNK(path_metadata.st_mode) or not stat.S_ISREG( + path_metadata.st_mode ): raise SetupOwnerError( - "The staged setup script changed while it was opened." + "The staged setup script must be a regular file." ) - content = stream.read(request.script.size + 1) + with path.open("rb") as stream: + opened_metadata = os.fstat(stream.fileno()) + if ( + not stat.S_ISREG(opened_metadata.st_mode) + or (path_metadata.st_dev, path_metadata.st_ino) + != (opened_metadata.st_dev, opened_metadata.st_ino) + ): + raise SetupOwnerError( + "The staged setup script changed while it was opened." + ) + content = stream.read(request.script.size + 1) + except SetupOwnerError as error: + if not isinstance(error.__cause__, FileNotFoundError): + raise + raise SetupOwnerError("The staged setup script is missing.") from error except FileNotFoundError as error: raise SetupOwnerError("The staged setup script is missing.") from error if ( @@ -481,8 +522,8 @@ def _parse_request(raw: bytes) -> SetupRequest: if not isinstance(raw, bytes) or not raw or len(raw) > MAX_REQUEST_BYTES: raise SetupOwnerError("Setup launch request must be 1-16 KiB.") try: - payload = json.loads(raw) - except (UnicodeDecodeError, json.JSONDecodeError) as error: + payload = _strict_json(raw) + except (UnicodeDecodeError, json.JSONDecodeError, ValueError) as error: raise SetupOwnerError( "Setup launch request is not valid UTF-8 JSON." ) from error @@ -554,20 +595,21 @@ def _spawn_keeper(directory: Path) -> None: flags = 0 if os.name == "nt": flags = subprocess.CREATE_NEW_PROCESS_GROUP | subprocess.DETACHED_PROCESS - process = subprocess.Popen( - [ - sys.executable, - "-m", - "blendersessiond.windows_setup_process", - "--keeper", - str(directory), - ], - stdin=subprocess.DEVNULL, - stdout=subprocess.DEVNULL, - stderr=subprocess.DEVNULL, - close_fds=True, - creationflags=flags, - ) + with _guard_setup_path(directory): + process = subprocess.Popen( + [ + sys.executable, + "-m", + "blendersessiond.windows_setup_process", + "--keeper", + str(directory), + ], + stdin=subprocess.DEVNULL, + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + close_fds=True, + creationflags=flags, + ) if wait_for_process_start_time(process.pid) is None: raise SetupOwnerError( "The setup keeper exited before ownership acknowledgement." @@ -599,12 +641,14 @@ def _load_request(directory: Path) -> SetupRequest: def _check_fence( request: SetupRequest, - request_sha256: str | None, - launch_id: str | None, + request_sha256: str, + launch_id: str, ) -> None: - if request_sha256 is not None and request_sha256 != request.request_sha256: + request_sha256 = _validate_sha256(request_sha256) + launch_id = _validate_launch_id(launch_id) + if request_sha256 != request.request_sha256: raise SetupOwnerError("The expected request SHA-256 does not match.") - if launch_id is not None and launch_id != request.launch_id: + if launch_id != request.launch_id: raise SetupOwnerError("The expected Launch ID does not match.") @@ -618,23 +662,11 @@ def _reconcile_after_stop( current = _read_view(directory, request) if current.terminal is not None: return current - keeper = process_matches(receipt.keeper_pid, receipt.keeper_creation_time) - root = process_matches(receipt.root_pid, receipt.root_creation_time) - if not keeper and not root and _job_is_absent(receipt.job_name): - return _publish_terminal( - directory, - request, - SetupTerminal.owned( - request=request, - outcome="stopped", - process="cancelled", - cleanup="tree_gone", - stdout=b"", - stderr=b"", - finished_at=datetime.now(UTC), - ), - ) time.sleep(0.05) + keeper = _process_state(receipt.keeper_pid, receipt.keeper_creation_time) + root = _process_state(receipt.root_pid, receipt.root_creation_time) + if keeper in {"matches", "unknown"} or root == "unknown": + return _unverified_view(request, receipt) return _publish_terminal( directory, request, @@ -657,25 +689,30 @@ def _reconcile_owner_loss( ) -> SetupView: deadline = time.monotonic() + _OWNER_LOSS_WAIT_SECONDS while time.monotonic() < deadline: - if process_matches(receipt.keeper_pid, receipt.keeper_creation_time): + keeper = _process_state(receipt.keeper_pid, receipt.keeper_creation_time) + if keeper == "matches": return SetupView(request=request, receipt=receipt) - if not process_matches(receipt.root_pid, receipt.root_creation_time): + if _process_is_gone( + _process_state(receipt.root_pid, receipt.root_creation_time) + ): break time.sleep(0.02) - root = process_matches(receipt.root_pid, receipt.root_creation_time) - cleanup = ( - "tree_gone" - if not root and _job_is_absent(receipt.job_name) - else "cleanup_unverified" - ) + keeper = _process_state(receipt.keeper_pid, receipt.keeper_creation_time) + if keeper == "matches": + return SetupView(request=request, receipt=receipt) + if keeper == "unknown": + return _unverified_view(request, receipt) + root = _process_state(receipt.root_pid, receipt.root_creation_time) + if root == "unknown": + return _unverified_view(request, receipt) return _publish_terminal( directory, request, SetupTerminal.owned( request=request, - outcome="owner_lost" if cleanup == "tree_gone" else cleanup, + outcome="cleanup_unverified", process="owner_lost", - cleanup=cleanup, + cleanup="cleanup_unverified", stdout=b"", stderr=b"", finished_at=datetime.now(UTC), @@ -688,7 +725,7 @@ def _publish_terminal( request: SetupRequest, terminal: SetupTerminal, ) -> SetupView: - with file_lock(directory / ".lock"): + with _setup_lock(directory): existing = _read_optional_record(directory / "terminal.json") if existing is not None: return SetupView( @@ -700,24 +737,26 @@ def _publish_terminal( def _write_create_once(path: Path, payload: dict[str, Any]) -> None: - encoded = _encode_record(payload) - path.parent.mkdir(mode=0o700, parents=True, exist_ok=True) - temporary = path.parent / f".{path.name}.{secrets.token_hex(16)}.tmp" - descriptor = os.open(temporary, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o600) - try: - with os.fdopen(descriptor, "wb", closefd=False) as stream: - stream.write(encoded) - stream.flush() - os.fsync(stream.fileno()) - finally: - os.close(descriptor) - try: - _publish_no_replace(temporary, path) - finally: + with _guard_setup_path(path.parent): + encoded = _encode_record(payload) + temporary = path.parent / f".{path.name}.{secrets.token_hex(16)}.tmp" + descriptor = os.open( + temporary, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o600 + ) try: - temporary.unlink() - except FileNotFoundError: - pass + with os.fdopen(descriptor, "wb", closefd=False) as stream: + stream.write(encoded) + stream.flush() + os.fsync(stream.fileno()) + finally: + os.close(descriptor) + try: + _publish_no_replace(temporary, path) + finally: + try: + temporary.unlink() + except FileNotFoundError: + pass def _publish_no_replace(source: Path, destination: Path) -> None: @@ -737,25 +776,27 @@ def _publish_no_replace(source: Path, destination: Path) -> None: def _touch_create_once(path: Path) -> bool: - try: - descriptor = os.open(path, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o600) - except FileExistsError: - return False - os.close(descriptor) - return True + with _guard_setup_path(path.parent): + try: + descriptor = os.open(path, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o600) + except FileExistsError: + return False + os.close(descriptor) + return True def _read_optional_record(path: Path) -> dict[str, Any] | None: - try: - with path.open("rb") as stream: - encoded = stream.read(MAX_RECORD_BYTES + 1) - except FileNotFoundError: - return None + with _guard_setup_path(path, allow_missing=True): + try: + with path.open("rb") as stream: + encoded = stream.read(MAX_RECORD_BYTES + 1) + except FileNotFoundError: + return None if len(encoded) > MAX_RECORD_BYTES: raise SetupOwnerError(f"Setup record is too large: {path.name}.") try: - payload = json.loads(encoded) - except (UnicodeDecodeError, json.JSONDecodeError) as error: + payload = _strict_json(encoded) + except (UnicodeDecodeError, json.JSONDecodeError, ValueError) as error: raise SetupOwnerError(f"Setup record is invalid: {path.name}.") from error if ( not isinstance(payload, dict) @@ -826,8 +867,9 @@ def _receipt_from_record( keeper_creation_time=payload["keeper_creation_time"], root_pid=payload["root_pid"], root_creation_time=payload["root_creation_time"], - job_name=payload["job_name"], + job_scope=payload.get("job_scope"), owned_at=_parse_time(payload["owned_at"], "owned_at"), + job_name=payload.get("job_name"), ) except (KeyError, TypeError) as error: raise SetupOwnerError("Setup launch receipt is invalid.") from error @@ -844,7 +886,13 @@ def _receipt_from_record( and not isinstance(receipt.root_pid, bool) and receipt.root_pid > 0 and isinstance(receipt.root_creation_time, str) - and receipt.job_name == _job_name(request.launch_id) + and ( + (receipt.job_scope == JOB_SCOPE and receipt.job_name is None) + or ( + receipt.job_scope is None + and receipt.job_name == _legacy_job_name(request.launch_id) + ) + ) ) if not valid: raise SetupOwnerError("Setup launch receipt is invalid or stale.") @@ -991,25 +1039,99 @@ def _format_time(value: datetime) -> str: return value.astimezone(UTC).isoformat().replace("+00:00", "Z") -def _require_windows(value: str | None) -> None: - if (platform.system() if value is None else value) != "Windows": - raise SetupOwnerError( - "Windows setup ownership is unavailable on this platform." - ) +def _strict_json(encoded: bytes) -> Any: + text = encoded.decode("utf-8", errors="strict") + if text.startswith("\ufeff"): + raise ValueError("UTF-8 BOM is not allowed.") + def object_from_pairs(pairs: list[tuple[str, Any]]) -> dict[str, Any]: + value: dict[str, Any] = {} + for key, item in pairs: + if key in value: + raise ValueError(f"Duplicate JSON key: {key}.") + value[key] = item + return value + + def reject_constant(value: str) -> None: + raise ValueError(f"Non-finite JSON value: {value}.") + + return json.loads( + text, + object_pairs_hook=object_from_pairs, + parse_constant=reject_constant, + ) + + +def _process_state(pid: int, creation_time: str) -> str: + if os.name == "nt": + from blendersessiond.windows_setup_process import exact_process_state + + return exact_process_state(pid, creation_time) + return "matches" if process_matches(pid, creation_time) else "absent" -def _job_name(launch_id: str) -> str: - return f"Local\\BlenderSessiond.Setup.{launch_id}" +def _process_is_gone(state: str) -> bool: + return state in {"absent", "different"} -def _job_exists(name: str) -> bool: - from blendersessiond.windows_setup_process import job_exists - return job_exists(name) +def _unverified_view( + request: SetupRequest, receipt: LaunchReceipt +) -> SetupView: + return SetupView( + request=request, + receipt=receipt, + ownership_unverified=True, + ) + + +def _legacy_job_name(launch_id: str) -> str: + return f"Local\\BlenderSessiond.Setup.{launch_id}" -def _job_is_absent(name: str) -> bool: +@contextmanager +def _guard_setup_path( + path: Path, *, allow_missing: bool = False +) -> Iterator[None]: + if os.name != "nt": + yield + return + from blendersessiond.windows_path_authority import ( + PathAuthorityError, + guard_path, + ) + + parts = path.parts + setup_indexes = [ + index for index, part in enumerate(parts) if part == "setup-attempts" + ] + authority_root = ( + path.__class__(*parts[: setup_indexes[-1]]) if setup_indexes else path + ) try: - return not _job_exists(name) - except (OSError, RuntimeError): - return False + with guard_path( + path, + authority_root=authority_root, + allow_missing=allow_missing, + ): + yield + except (OSError, PathAuthorityError) as error: + raise SetupOwnerError(str(error)) from error + + +@contextmanager +def _setup_lock(directory: Path) -> Iterator[None]: + with _guard_setup_path(directory): + with file_lock(directory / ".lock"): + yield + + +def _marker_exists(path: Path) -> bool: + with _guard_setup_path(path, allow_missing=True): + return path.exists() + + +def _require_windows(value: str | None) -> None: + if (platform.system() if value is None else value) != "Windows": + raise SetupOwnerError( + "Windows setup ownership is unavailable on this platform." + ) diff --git a/src/blendersessiond/windows_path_authority.py b/src/blendersessiond/windows_path_authority.py new file mode 100644 index 0000000..4c42070 --- /dev/null +++ b/src/blendersessiond/windows_path_authority.py @@ -0,0 +1,453 @@ +from __future__ import annotations + +import ctypes +from collections.abc import Iterator +from contextlib import contextmanager +from ctypes import wintypes +from dataclasses import dataclass +from pathlib import PureWindowsPath +from typing import Any + +FILE_WRITE_DATA = 0x0002 +_FILE_APPEND_DATA = 0x0004 +_FILE_WRITE_EA = 0x0010 +_FILE_DELETE_CHILD = 0x0040 +_FILE_WRITE_ATTRIBUTES = 0x0100 +_DELETE = 0x00010000 +_WRITE_DAC = 0x00040000 +_WRITE_OWNER = 0x00080000 +_GENERIC_ALL = 0x10000000 +_GENERIC_WRITE = 0x40000000 +_DANGEROUS_ACCESS = ( + FILE_WRITE_DATA + | _FILE_APPEND_DATA + | _FILE_WRITE_EA + | _FILE_DELETE_CHILD + | _FILE_WRITE_ATTRIBUTES + | _DELETE + | _WRITE_DAC + | _WRITE_OWNER + | _GENERIC_ALL + | _GENERIC_WRITE +) + +_READ_CONTROL = 0x00020000 +_FILE_READ_ATTRIBUTES = 0x0080 +_FILE_SHARE_READ = 0x00000001 +_FILE_SHARE_WRITE = 0x00000002 +_OPEN_EXISTING = 3 +_FILE_FLAG_BACKUP_SEMANTICS = 0x02000000 +_FILE_FLAG_OPEN_REPARSE_POINT = 0x00200000 +_FILE_ATTRIBUTE_REPARSE_POINT = 0x00000400 +_FILE_ATTRIBUTE_TAG_INFO = 9 +_SE_FILE_OBJECT = 1 +_OWNER_SECURITY_INFORMATION = 0x00000001 +_DACL_SECURITY_INFORMATION = 0x00000004 +_SE_DACL_PROTECTED = 0x1000 +_TOKEN_QUERY = 0x0008 +_TOKEN_USER = 1 +_SECURITY_MAX_SID_SIZE = 68 +_WIN_CREATOR_OWNER_SID = 3 +_WIN_LOCAL_SYSTEM_SID = 22 +_WIN_BUILTIN_ADMINISTRATORS_SID = 26 +_ACCESS_ALLOWED_ACE_TYPES = {0, 5, 9, 11} +_UNSUPPORTED_ACCESS_ALLOWED_ACE_TYPES = {4} +_ACE_OBJECT_TYPE_PRESENT = 0x1 +_ACE_INHERITED_OBJECT_TYPE_PRESENT = 0x2 + + +class PathAuthorityError(RuntimeError): + pass + + +@dataclass(frozen=True) +class AccessEntry: + sid: bytes | None + mask: int + + +@dataclass(frozen=True) +class PathSecurity: + owner_sid: bytes | None + dacl_present: bool + aces: tuple[AccessEntry, ...] + dacl_protected: bool = True + + +class _FileAttributeTagInfo(ctypes.Structure): + _fields_ = [ + ("file_attributes", wintypes.DWORD), + ("reparse_tag", wintypes.DWORD), + ] + + +class _Acl(ctypes.Structure): + _fields_ = [ + ("revision", ctypes.c_ubyte), + ("reserved", ctypes.c_ubyte), + ("size", wintypes.WORD), + ("ace_count", wintypes.WORD), + ("reserved2", wintypes.WORD), + ] + + +class _AceHeader(ctypes.Structure): + _fields_ = [ + ("ace_type", ctypes.c_ubyte), + ("ace_flags", ctypes.c_ubyte), + ("ace_size", wintypes.WORD), + ] + + +class _TokenUser(ctypes.Structure): + _fields_ = [("sid", wintypes.LPVOID), ("attributes", wintypes.DWORD)] + + +class _NativePathApi: + def __init__(self) -> None: + self.kernel = _kernel32() + self.advapi = _advapi32() + + def open_path(self, path: str) -> int: + handle = self.kernel.CreateFileW( + path, + _READ_CONTROL | _FILE_READ_ATTRIBUTES, + _FILE_SHARE_READ | _FILE_SHARE_WRITE, + None, + _OPEN_EXISTING, + _FILE_FLAG_BACKUP_SEMANTICS | _FILE_FLAG_OPEN_REPARSE_POINT, + None, + ) + if handle == wintypes.HANDLE(-1).value: + raise ctypes.WinError(ctypes.get_last_error()) + return handle + + def is_reparse_point(self, handle: int) -> bool: + information = _FileAttributeTagInfo() + if not self.kernel.GetFileInformationByHandleEx( + handle, + _FILE_ATTRIBUTE_TAG_INFO, + ctypes.byref(information), + ctypes.sizeof(information), + ): + raise ctypes.WinError(ctypes.get_last_error()) + return bool(information.file_attributes & _FILE_ATTRIBUTE_REPARSE_POINT) + + def read_security(self, handle: int) -> PathSecurity: + owner = wintypes.LPVOID() + dacl = wintypes.LPVOID() + descriptor = wintypes.LPVOID() + result = self.advapi.GetSecurityInfo( + handle, + _SE_FILE_OBJECT, + _OWNER_SECURITY_INFORMATION | _DACL_SECURITY_INFORMATION, + ctypes.byref(owner), + None, + ctypes.byref(dacl), + None, + ctypes.byref(descriptor), + ) + if result: + raise ctypes.WinError(result) + try: + control = wintypes.WORD() + revision = wintypes.DWORD() + if not self.advapi.GetSecurityDescriptorControl( + descriptor, ctypes.byref(control), ctypes.byref(revision) + ): + raise ctypes.WinError(ctypes.get_last_error()) + return PathSecurity( + owner_sid=self._sid_bytes(owner.value) if owner.value else None, + dacl_present=bool(dacl.value), + aces=self._access_entries(dacl.value) if dacl.value else (), + dacl_protected=bool(control.value & _SE_DACL_PROTECTED), + ) + finally: + if descriptor.value: + self.kernel.LocalFree(descriptor) + + def trusted_sids(self) -> frozenset[bytes]: + return frozenset( + { + self._current_user_sid(), + self._well_known_sid(_WIN_CREATOR_OWNER_SID), + self._well_known_sid(_WIN_LOCAL_SYSTEM_SID), + self._well_known_sid(_WIN_BUILTIN_ADMINISTRATORS_SID), + } + ) + + def close_handle(self, handle: int) -> None: + self.kernel.CloseHandle(handle) + + def _current_user_sid(self) -> bytes: + token = wintypes.HANDLE() + if not self.advapi.OpenProcessToken( + self.kernel.GetCurrentProcess(), _TOKEN_QUERY, ctypes.byref(token) + ): + raise ctypes.WinError(ctypes.get_last_error()) + try: + size = wintypes.DWORD() + self.advapi.GetTokenInformation( + token, _TOKEN_USER, None, 0, ctypes.byref(size) + ) + if not size.value: + raise ctypes.WinError(ctypes.get_last_error()) + buffer = ctypes.create_string_buffer(size.value) + if not self.advapi.GetTokenInformation( + token, + _TOKEN_USER, + buffer, + size, + ctypes.byref(size), + ): + raise ctypes.WinError(ctypes.get_last_error()) + user = ctypes.cast(buffer, ctypes.POINTER(_TokenUser)).contents + return self._sid_bytes(user.sid) + finally: + self.close_handle(token.value) + + def _well_known_sid(self, sid_type: int) -> bytes: + buffer = ctypes.create_string_buffer(_SECURITY_MAX_SID_SIZE) + size = wintypes.DWORD(len(buffer)) + if not self.advapi.CreateWellKnownSid( + sid_type, None, buffer, ctypes.byref(size) + ): + raise ctypes.WinError(ctypes.get_last_error()) + return bytes(buffer.raw[: size.value]) + + def _sid_bytes(self, sid: int) -> bytes: + if not self.advapi.IsValidSid(sid): + raise PathAuthorityError("Setup path contains an invalid security SID.") + size = self.advapi.GetLengthSid(sid) + if not 0 < size <= _SECURITY_MAX_SID_SIZE: + raise PathAuthorityError("Setup path contains an invalid security SID.") + return ctypes.string_at(sid, size) + + def _access_entries(self, dacl: int) -> tuple[AccessEntry, ...]: + header = ctypes.cast(dacl, ctypes.POINTER(_Acl)).contents + entries: list[AccessEntry] = [] + for index in range(header.ace_count): + ace = wintypes.LPVOID() + if not self.advapi.GetAce(dacl, index, ctypes.byref(ace)): + raise ctypes.WinError(ctypes.get_last_error()) + address = ace.value + ace_header = ctypes.cast(address, ctypes.POINTER(_AceHeader)).contents + if ace_header.ace_type in _UNSUPPORTED_ACCESS_ALLOWED_ACE_TYPES: + raise PathAuthorityError( + "Setup authority path contains an unsupported access-allowed ACE." + ) + if ace_header.ace_type not in _ACCESS_ALLOWED_ACE_TYPES: + continue + mask = ctypes.cast( + address + ctypes.sizeof(_AceHeader), ctypes.POINTER(wintypes.DWORD) + ).contents.value + sid_address = self._allowed_ace_sid_address( + address, ace_header.ace_type + ) + entries.append( + AccessEntry( + sid=self._sid_bytes(sid_address) if sid_address else None, + mask=mask, + ) + ) + return tuple(entries) + + @staticmethod + def _allowed_ace_sid_address(address: int, ace_type: int) -> int | None: + if ace_type in {0, 9}: + return address + ctypes.sizeof(_AceHeader) + ctypes.sizeof(wintypes.DWORD) + flags_address = ( + address + ctypes.sizeof(_AceHeader) + ctypes.sizeof(wintypes.DWORD) + ) + object_flags = ctypes.cast( + flags_address, ctypes.POINTER(wintypes.DWORD) + ).contents.value + offset = ( + ctypes.sizeof(_AceHeader) + + ctypes.sizeof(wintypes.DWORD) + + ctypes.sizeof(wintypes.DWORD) + ) + if object_flags & _ACE_OBJECT_TYPE_PRESENT: + offset += 16 + if object_flags & _ACE_INHERITED_OBJECT_TYPE_PRESENT: + offset += 16 + return address + offset + + +@contextmanager +def guard_path( + path: PureWindowsPath, + *, + authority_root: PureWindowsPath | None = None, + allow_missing: bool = False, + api: Any | None = None, +) -> Iterator[None]: + candidate = PureWindowsPath(path) + authority = ( + candidate if authority_root is None else PureWindowsPath(authority_root) + ) + if not candidate.is_absolute() or not candidate.anchor: + raise PathAuthorityError("Setup authority paths must be absolute.") + if ( + not authority.is_absolute() + or ".." in candidate.parts + or ".." in authority.parts + ): + raise PathAuthorityError("Setup authority paths cannot contain '..'.") + components = _path_components(candidate) + try: + authority_index = components.index(authority) + except ValueError as error: + raise PathAuthorityError( + "Setup authority root must contain the protected path." + ) from error + owner = _NativePathApi() if api is None else api + handles: list[object] = [] + try: + for component in components: + try: + handle = owner.open_path(str(component)) + except FileNotFoundError: + if allow_missing: + break + raise + handles.append(handle) + if owner.is_reparse_point(handle): + raise PathAuthorityError( + "Setup authority paths cannot contain reparse points." + ) + if not handles: + raise PathAuthorityError("Setup authority path has no existing anchor.") + if authority_index >= len(handles): + raise PathAuthorityError( + "Setup authority root must exist before staging." + ) + if not allow_missing and len(handles) != len(components): + raise FileNotFoundError(str(candidate)) + trusted = owner.trusted_sids() + _validate_security( + owner.read_security(handles[authority_index]), + trusted, + require_protected=True, + ) + if authority_index != len(handles) - 1: + _validate_security( + owner.read_security(handles[-1]), + trusted, + require_protected=False, + ) + yield + finally: + for handle in reversed(handles): + owner.close_handle(handle) + + +def _path_components(path: PureWindowsPath) -> tuple[PureWindowsPath, ...]: + current = PureWindowsPath(path.anchor) + components = [current] + for part in path.parts[1:]: + current /= part + components.append(current) + return tuple(components) + + +def _validate_security( + security: PathSecurity, + trusted_sids: frozenset[bytes], + *, + require_protected: bool, +) -> None: + if security.owner_sid not in trusted_sids: + raise PathAuthorityError("Setup authority path has an untrusted owner.") + if not security.dacl_present: + raise PathAuthorityError("Setup authority path must have a non-null DACL.") + if require_protected and not security.dacl_protected: + raise PathAuthorityError( + "Setup authority root must disable inherited permissions." + ) + for entry in security.aces: + if entry.mask & _DANGEROUS_ACCESS and entry.sid not in trusted_sids: + raise PathAuthorityError( + "Setup authority path grants write access to an untrusted principal." + ) + + +def _kernel32(): + kernel = ctypes.WinDLL("kernel32", use_last_error=True) + kernel.CreateFileW.argtypes = [ + wintypes.LPCWSTR, + wintypes.DWORD, + wintypes.DWORD, + wintypes.LPVOID, + wintypes.DWORD, + wintypes.DWORD, + wintypes.HANDLE, + ] + kernel.CreateFileW.restype = wintypes.HANDLE + kernel.GetFileInformationByHandleEx.argtypes = [ + wintypes.HANDLE, + ctypes.c_int, + wintypes.LPVOID, + wintypes.DWORD, + ] + kernel.GetFileInformationByHandleEx.restype = wintypes.BOOL + kernel.CloseHandle.argtypes = [wintypes.HANDLE] + kernel.CloseHandle.restype = wintypes.BOOL + kernel.LocalFree.argtypes = [wintypes.LPVOID] + kernel.LocalFree.restype = wintypes.LPVOID + kernel.GetCurrentProcess.argtypes = [] + kernel.GetCurrentProcess.restype = wintypes.HANDLE + return kernel + + +def _advapi32(): + advapi = ctypes.WinDLL("advapi32", use_last_error=True) + advapi.GetSecurityInfo.argtypes = [ + wintypes.HANDLE, + ctypes.c_int, + wintypes.DWORD, + ctypes.POINTER(wintypes.LPVOID), + ctypes.POINTER(wintypes.LPVOID), + ctypes.POINTER(wintypes.LPVOID), + ctypes.POINTER(wintypes.LPVOID), + ctypes.POINTER(wintypes.LPVOID), + ] + advapi.GetSecurityInfo.restype = wintypes.DWORD + advapi.GetSecurityDescriptorControl.argtypes = [ + wintypes.LPVOID, + ctypes.POINTER(wintypes.WORD), + ctypes.POINTER(wintypes.DWORD), + ] + advapi.GetSecurityDescriptorControl.restype = wintypes.BOOL + advapi.OpenProcessToken.argtypes = [ + wintypes.HANDLE, + wintypes.DWORD, + ctypes.POINTER(wintypes.HANDLE), + ] + advapi.OpenProcessToken.restype = wintypes.BOOL + advapi.GetTokenInformation.argtypes = [ + wintypes.HANDLE, + ctypes.c_int, + wintypes.LPVOID, + wintypes.DWORD, + ctypes.POINTER(wintypes.DWORD), + ] + advapi.GetTokenInformation.restype = wintypes.BOOL + advapi.CreateWellKnownSid.argtypes = [ + ctypes.c_int, + wintypes.LPVOID, + wintypes.LPVOID, + ctypes.POINTER(wintypes.DWORD), + ] + advapi.CreateWellKnownSid.restype = wintypes.BOOL + advapi.IsValidSid.argtypes = [wintypes.LPVOID] + advapi.IsValidSid.restype = wintypes.BOOL + advapi.GetLengthSid.argtypes = [wintypes.LPVOID] + advapi.GetLengthSid.restype = wintypes.DWORD + advapi.GetAce.argtypes = [ + wintypes.LPVOID, + wintypes.DWORD, + ctypes.POINTER(wintypes.LPVOID), + ] + advapi.GetAce.restype = wintypes.BOOL + return advapi diff --git a/src/blendersessiond/windows_setup_process.py b/src/blendersessiond/windows_setup_process.py index 16bf92e..43a866e 100644 --- a/src/blendersessiond/windows_setup_process.py +++ b/src/blendersessiond/windows_setup_process.py @@ -14,7 +14,6 @@ from pathlib import Path from typing import Any -from blendersessiond.locking import file_lock from blendersessiond.processes import process_start_time CREATE_SUSPENDED = 0x00000004 @@ -30,8 +29,7 @@ _WAIT_OBJECT_0 = 0 _WAIT_TIMEOUT = 258 _ERROR_BROKEN_PIPE = 109 -_ERROR_ALREADY_EXISTS = 183 -_ERROR_FILE_NOT_FOUND = 2 +_ERROR_INVALID_PARAMETER = 87 _STILL_ACTIVE = 259 _MAX_STREAM_BYTES = 24 * 1024 @@ -139,14 +137,10 @@ class _NativeWin32: def __init__(self) -> None: self.kernel = _kernel32() - def create_job(self, name: str | None) -> int: - job = self.kernel.CreateJobObjectW(None, name) - error = ctypes.get_last_error() + def create_job(self) -> int: + job = self.kernel.CreateJobObjectW(None, None) if not job: - raise ctypes.WinError(error) - if name is not None and error == _ERROR_ALREADY_EXISTS: - self.close_handles((job,)) - raise RuntimeError("Setup Job Object name is already in use.") + raise ctypes.WinError(ctypes.get_last_error()) return job def set_kill_on_close(self, job: int) -> None: @@ -358,7 +352,6 @@ def create_suspended( *, api: Any | None = None, powershell_path: Path | None = None, - job_name: str | None, script_size: int = 1, script_sha256: str = "0" * 64, cwd: Path | None = None, @@ -367,7 +360,7 @@ def create_suspended( powershell = ( _system_powershell() if powershell_path is None else str(powershell_path) ) - job = owner.create_job(job_name) + job = owner.create_job() try: owner.set_kill_on_close(job) child, parent = owner.create_standard_pipes() @@ -543,17 +536,16 @@ def powershell_bootstrap(script_size: int, script_sha256: str) -> str: def runtime_self_test() -> None: owned = WindowsSetupProcess.create_suspended( - job_name=None, script_size=1, script_sha256="0" * 64, ) try: if owned.active_count() != 1: raise RuntimeError("Setup owner self-test found unexpected Job members.") - owned.terminate() - owned.wait_empty() - if not owned.empty(): - raise RuntimeError("Setup owner self-test did not empty the Job Object.") + owned._api.close_handles((owned.job_handle,)) + owned.job_handle = 0 + if owned._api.wait_process(owned.process_handle, 5000) != _WAIT_OBJECT_0: + raise RuntimeError("Setup owner self-test did not enforce kill-on-close.") finally: owned.close() @@ -562,7 +554,10 @@ def terminate_exact_process(pid: int, expected_creation_time: str) -> None: api = _NativeWin32() process = api.kernel.OpenProcess(0x00100000 | 0x1000 | 0x0001, False, pid) if not process: - return + error = ctypes.get_last_error() + if error == _ERROR_INVALID_PARAMETER: + return + raise ctypes.WinError(error) try: actual = f"windows:{api.process_creation_filetime(process)}" if actual != expected_creation_time: @@ -574,21 +569,48 @@ def terminate_exact_process(pid: int, expected_creation_time: str) -> None: api.close_handles((process,)) -def job_exists(name: str) -> bool: - kernel = _kernel32() - job = kernel.OpenJobObjectW(0x0004, False, name) - if not job: - error = ctypes.get_last_error() - if error == _ERROR_FILE_NOT_FOUND: - return False - raise ctypes.WinError(error) - kernel.CloseHandle(job) - return True +def exact_process_state( + pid: int, + expected_creation_time: str, + *, + api: Any | None = None, +) -> str: + owner = _NativeWin32() if api is None else api + process = owner.kernel.OpenProcess(0x00100000 | 0x1000, False, pid) + if not process: + return ( + "absent" + if ctypes.get_last_error() == _ERROR_INVALID_PARAMETER + else "unknown" + ) + try: + try: + actual = f"windows:{owner.process_creation_filetime(process)}" + except OSError: + return "unknown" + if actual != expected_creation_time: + return "different" + wait = owner.wait_process(process, 0) + if wait == _WAIT_OBJECT_0: + return "absent" + return "matches" if wait == _WAIT_TIMEOUT else "unknown" + finally: + owner.close_handles((process,)) def run_keeper(attempt_dir: Path) -> int: from blendersessiond import setup_owner + try: + with setup_owner._guard_setup_path(attempt_dir): + return _run_keeper(attempt_dir) + except BaseException: + return 2 + + +def _run_keeper(attempt_dir: Path) -> int: + from blendersessiond import setup_owner + try: request = setup_owner._load_request(attempt_dir) except BaseException: @@ -613,7 +635,7 @@ def run_keeper(attempt_dir: Path) -> int: threads: list[threading.Thread] = [] resumed = False try: - with file_lock(attempt_dir / ".lock"): + with setup_owner._setup_lock(attempt_dir): if (attempt_dir / "terminal.json").exists(): return 0 if (attempt_dir / ".stop").exists(): @@ -631,7 +653,6 @@ def run_keeper(attempt_dir: Path) -> int: ) return 0 owned = WindowsSetupProcess.create_suspended( - job_name=setup_owner._job_name(request.launch_id), script_size=request.script.size, script_sha256=request.script.sha256, cwd=attempt_dir, @@ -647,10 +668,10 @@ def run_keeper(attempt_dir: Path) -> int: keeper_creation_time=keeper_time, root_pid=owned.root_pid, root_creation_time=f"windows:{owned.root_creation_filetime}", - job_name=setup_owner._job_name(request.launch_id), + job_scope=setup_owner.JOB_SCOPE, owned_at=datetime.now(UTC), ) - with file_lock(attempt_dir / ".lock"): + with setup_owner._setup_lock(attempt_dir): if (attempt_dir / "terminal.json").exists(): owned.terminate() owned.wait_empty() @@ -702,6 +723,8 @@ def run_keeper(attempt_dir: Path) -> int: owned.finish_io(threads) stdout, stdout_truncated = output.get("stdout", (b"", False)) stderr, stderr_truncated = output.get("stderr", (b"", False)) + stdout = _valid_utf8_prefix(stdout, truncated=stdout_truncated) + stderr = _valid_utf8_prefix(stderr, truncated=stderr_truncated) stdout.decode("utf-8", errors="strict") stderr.decode("utf-8", errors="strict") exit_value = None if exit_code == _STILL_ACTIVE else exit_code @@ -775,7 +798,12 @@ def _wait_for_process( min(100, max(1, int(wait_seconds * 1000))), ) if result == _WAIT_OBJECT_0: - return _wait_for_owned_job(owned, deadline_utc, stop_path) + return _wait_for_owned_job( + owned, + deadline_utc, + stop_path, + monotonic_deadline=deadline, + ) if result != _WAIT_TIMEOUT: raise ctypes.WinError(ctypes.get_last_error()) @@ -784,12 +812,20 @@ def _wait_for_owned_job( owned: WindowsSetupProcess, deadline_utc: datetime, stop_path: Path, + *, + monotonic_deadline: float | None = None, ) -> str: + if monotonic_deadline is None: + remaining = max(0.0, (deadline_utc - datetime.now(UTC)).total_seconds()) + monotonic_deadline = time.monotonic() + min(remaining, 5 * 60) while True: if stop_path.exists(): owned.terminate() return "cancelled" - if datetime.now(UTC) >= deadline_utc: + if ( + datetime.now(UTC) >= deadline_utc + or time.monotonic() >= monotonic_deadline + ): if not owned.empty(): owned.terminate() return "timed_out" @@ -798,6 +834,18 @@ def _wait_for_owned_job( time.sleep(0.02) +def _valid_utf8_prefix(content: bytes, *, truncated: bool) -> bytes: + try: + content.decode("utf-8", errors="strict") + return content + except UnicodeDecodeError as error: + if not truncated: + raise + prefix = content[: error.start] + prefix.decode("utf-8", errors="strict") + return prefix + + def _powershell_arguments(script_size: int, script_sha256: str) -> list[str]: encoded = base64.b64encode( powershell_bootstrap(script_size, script_sha256).encode("utf-16-le") @@ -933,12 +981,6 @@ def _kernel32(): kernel.WriteFile.restype = wintypes.BOOL kernel.OpenProcess.argtypes = [wintypes.DWORD, wintypes.BOOL, wintypes.DWORD] kernel.OpenProcess.restype = wintypes.HANDLE - kernel.OpenJobObjectW.argtypes = [ - wintypes.DWORD, - wintypes.BOOL, - wintypes.LPCWSTR, - ] - kernel.OpenJobObjectW.restype = wintypes.HANDLE kernel.TerminateProcess.argtypes = [wintypes.HANDLE, wintypes.UINT] kernel.TerminateProcess.restype = wintypes.BOOL kernel.CloseHandle.argtypes = [wintypes.HANDLE] diff --git a/tests/conftest.py b/tests/conftest.py index 16ca938..12f25ff 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -6,11 +6,13 @@ import subprocess import sys from collections.abc import Iterator +from contextlib import contextmanager from dataclasses import dataclass from pathlib import Path import pytest +from blendersessiond import setup_owner from blendersessiond.processes import ( owned_tree_exists, terminate_owned_tree, @@ -25,6 +27,18 @@ class CliResult: payload: dict +@pytest.fixture +def bypass_setup_path_authority(monkeypatch): + guarded = setup_owner._guard_setup_path + + @contextmanager + def unguarded(*_args, **_kwargs): + yield + + monkeypatch.setattr(setup_owner, "_guard_setup_path", unguarded) + return guarded + + def _free_mcp_port_base(span: int = 8) -> int: # Sessions get base, base+1, ... so a fixed base (9876) collides with any # real Session running on the developer machine; probe an ephemeral base diff --git a/tests/test_cli.py b/tests/test_cli.py index a80d11b..82094ff 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -75,6 +75,7 @@ def fake_stop(attempt_id: str, **kwargs): captured["attempt_id"] = attempt_id captured.update(kwargs) return SimpleNamespace( + terminal=SimpleNamespace(cleanup="tree_gone"), to_dict=lambda: {"schema_version": 1, "status": "terminal"} ) @@ -106,6 +107,78 @@ def fake_stop(attempt_id: str, **kwargs): assert json.loads(capsys.readouterr().out)["status"] == "terminal" +@pytest.mark.parametrize( + ("command", "terminal", "expected"), + [ + ("launch", SimpleNamespace(succeeded=False, cleanup="tree_gone"), 1), + ("status", SimpleNamespace(succeeded=False, cleanup="tree_gone"), 1), + ("stop", SimpleNamespace(succeeded=False, cleanup="tree_gone"), 0), + ("stop", SimpleNamespace(succeeded=False, cleanup="cleanup_unverified"), 1), + ], +) +def test_setup_owner_terminal_controls_exit_status( + command: str, + terminal: SimpleNamespace, + expected: int, + monkeypatch, + capsys, +) -> None: + result = SimpleNamespace( + terminal=terminal, + to_dict=lambda: {"schema_version": 1, "status": "terminal"}, + ) + attempt_id = "bbsa_" + "A" * 43 + launch_id = "bbsl_" + "B" * 43 + if command == "launch": + monkeypatch.setattr(cli, "launch_setup", lambda _raw: result) + monkeypatch.setattr( + cli.sys, "stdin", SimpleNamespace(buffer=io.BytesIO(b"{}")) + ) + arguments = ["setup-owner", "launch", "--json"] + else: + monkeypatch.setattr(cli, f"{command}_setup", lambda *_args, **_kwargs: result) + arguments = [ + "setup-owner", + command, + "--attempt-id", + attempt_id, + "--expect-request-sha256", + "c" * 64, + "--expect-launch-id", + launch_id, + "--json", + ] + + assert cli.main(arguments) == expected + assert json.loads(capsys.readouterr().out)["status"] == "terminal" + + +def test_setup_owner_unverified_view_returns_failure(monkeypatch, capsys) -> None: + result = SimpleNamespace( + terminal=None, + ownership_unverified=True, + to_dict=lambda: {"schema_version": 1, "status": "ownership_unverified"}, + ) + monkeypatch.setattr(cli, "status_setup", lambda *_args, **_kwargs: result) + assert ( + cli.main( + [ + "setup-owner", + "status", + "--attempt-id", + "bbsa_" + "A" * 43, + "--expect-request-sha256", + "c" * 64, + "--expect-launch-id", + "bbsl_" + "B" * 43, + "--json", + ] + ) + == 1 + ) + assert json.loads(capsys.readouterr().out)["status"] == "ownership_unverified" + + def test_setup_owner_parser_rejects_generic_process_inputs() -> None: parser = cli.build_parser() for option in ("--state-root", "--executable", "--args", "--env", "--cwd"): diff --git a/tests/test_setup_owner.py b/tests/test_setup_owner.py index 141e846..3ea9896 100644 --- a/tests/test_setup_owner.py +++ b/tests/test_setup_owner.py @@ -3,6 +3,7 @@ import hashlib import json import threading +from contextlib import contextmanager from datetime import UTC, datetime, timedelta from pathlib import Path @@ -10,6 +11,8 @@ from blendersessiond import setup_owner +pytestmark = pytest.mark.usefixtures("bypass_setup_path_authority") + ATTEMPT_ID = "bbsa_" + "A" * 43 LAUNCH_ID = "bbsl_" + "B" * 43 SCRIPT = b"Write-Output 'owned bytes'\n" @@ -101,6 +104,7 @@ def test_byte_identical_replay_remains_valid_after_deadline(tmp_path: Path) -> N ), "artifact_id", ), + (lambda value: value | {"deadline_utc": float("nan")}, "UTF-8 JSON"), ], ) def test_request_is_strictly_bounded(tmp_path: Path, mutate, message: str) -> None: @@ -114,6 +118,21 @@ def test_request_is_strictly_bounded(tmp_path: Path, mutate, message: str) -> No ) +@pytest.mark.parametrize( + "raw", + [ + b'{"schema_version":1,"schema_version":1}', + raw_launch(datetime(2026, 9, 3, 12, tzinfo=UTC)).decode().encode("utf-16"), + b"\xef\xbb\xbf{}", + ], +) +def test_request_rejects_duplicate_or_non_utf8_json( + tmp_path: Path, raw: bytes +) -> None: + with pytest.raises(setup_owner.SetupOwnerError, match="UTF-8 JSON"): + setup_owner.accept_launch_request(raw, state_root=tmp_path) + + def test_stop_before_dispatch_requires_full_fence_and_is_immutable( tmp_path: Path, ) -> None: @@ -285,7 +304,7 @@ def dispatch(directory: Path) -> None: keeper_creation_time="windows:10", root_pid=11, root_creation_time="windows:11", - job_name=setup_owner._job_name(request.launch_id), + job_scope=setup_owner.JOB_SCOPE, owned_at=now, ) setup_owner._write_create_once( @@ -327,7 +346,7 @@ def dispatch(directory: Path) -> None: keeper_creation_time="windows:10", root_pid=11, root_creation_time="windows:11", - job_name=setup_owner._job_name(request.launch_id), + job_scope=setup_owner.JOB_SCOPE, owned_at=now, ) setup_owner._write_create_once( @@ -346,6 +365,54 @@ def dispatch(directory: Path) -> None: assert result.terminal is None +def test_spawn_failure_is_an_immutable_terminal(tmp_path: Path, monkeypatch) -> None: + now = datetime(2026, 9, 3, 12, tzinfo=UTC) + stage_script(tmp_path) + monkeypatch.setattr( + setup_owner, + "_spawn_keeper", + lambda _directory: (_ for _ in ()).throw(OSError("spawn failed")), + ) + first = setup_owner.launch_setup( + raw_launch(now), + state_root=tmp_path, + now=now, + platform_name="Windows", + ) + second = setup_owner.launch_setup( + raw_launch(now), + state_root=tmp_path, + now=now, + platform_name="Windows", + ) + assert first == second + assert first.terminal.outcome == "launch_failed" + assert first.terminal.cleanup == "cleanup_unverified" + + +def test_status_requires_well_formed_complete_fences(tmp_path: Path) -> None: + now = datetime(2026, 9, 3, 12, tzinfo=UTC) + request = setup_owner.accept_launch_request( + raw_launch(now), state_root=tmp_path, now=now + ) + with pytest.raises(setup_owner.SetupOwnerError, match="SHA-256"): + setup_owner.status_setup( + ATTEMPT_ID, + expected_request_sha256="short", + expected_launch_id=LAUNCH_ID, + state_root=tmp_path, + platform_name="Windows", + ) + with pytest.raises(setup_owner.SetupOwnerError, match="Launch ID"): + setup_owner.status_setup( + ATTEMPT_ID, + expected_request_sha256=request.request_sha256, + expected_launch_id="short", + state_root=tmp_path, + platform_name="Windows", + ) + + def test_only_three_json_records_define_attempt_state(tmp_path: Path) -> None: now = datetime(2026, 9, 3, 12, tzinfo=UTC) stage_script(tmp_path) @@ -362,7 +429,7 @@ def test_only_three_json_records_define_attempt_state(tmp_path: Path) -> None: assert names <= {"request.json", "launch-receipt.json", "terminal.json"} -def test_owner_loss_is_persisted_only_after_tree_and_job_are_absent( +def test_unnamed_job_owner_loss_never_claims_tree_gone( tmp_path: Path, monkeypatch ) -> None: now = datetime(2026, 9, 3, 12, tzinfo=UTC) @@ -378,12 +445,11 @@ def test_owner_loss_is_persisted_only_after_tree_and_job_are_absent( keeper_creation_time="windows:10", root_pid=11, root_creation_time="windows:11", - job_name=setup_owner._job_name(LAUNCH_ID), + job_scope=setup_owner.JOB_SCOPE, owned_at=now, ) setup_owner._write_create_once(directory / "launch-receipt.json", receipt.to_dict()) monkeypatch.setattr(setup_owner, "process_matches", lambda *_args: False) - monkeypatch.setattr(setup_owner, "_job_exists", lambda _name: False) first = setup_owner.status_setup( ATTEMPT_ID, @@ -400,8 +466,8 @@ def test_owner_loss_is_persisted_only_after_tree_and_job_are_absent( platform_name="Windows", ) assert first == second - assert first.terminal.outcome == "owner_lost" - assert first.terminal.cleanup == "tree_gone" + assert first.terminal.outcome == "cleanup_unverified" + assert first.terminal.cleanup == "cleanup_unverified" def test_pid_reuse_is_safe(tmp_path: Path, monkeypatch) -> None: @@ -418,12 +484,11 @@ def test_pid_reuse_is_safe(tmp_path: Path, monkeypatch) -> None: keeper_creation_time="windows:old", root_pid=11, root_creation_time="windows:old-root", - job_name=setup_owner._job_name(LAUNCH_ID), + job_scope=setup_owner.JOB_SCOPE, owned_at=now, ) setup_owner._write_create_once(directory / "launch-receipt.json", receipt.to_dict()) monkeypatch.setattr(setup_owner, "process_matches", lambda *_args: False) - monkeypatch.setattr(setup_owner, "_job_exists", lambda _name: True) monkeypatch.setattr(setup_owner, "_STOP_WAIT_SECONDS", 0) from blendersessiond import windows_setup_process @@ -444,8 +509,9 @@ def test_pid_reuse_is_safe(tmp_path: Path, monkeypatch) -> None: assert result.terminal.cleanup == "cleanup_unverified" -def test_owner_loss_job_query_error_is_cleanup_unverified( - tmp_path: Path, monkeypatch +@pytest.mark.parametrize("operation", ["status", "stop"]) +def test_legacy_named_job_receipt_recovery_is_unverified( + operation: str, tmp_path: Path, monkeypatch ) -> None: now = datetime(2026, 9, 3, 12, tzinfo=UTC) request = setup_owner.accept_launch_request( @@ -460,18 +526,20 @@ def test_owner_loss_job_query_error_is_cleanup_unverified( keeper_creation_time="windows:10", root_pid=11, root_creation_time="windows:11", - job_name=setup_owner._job_name(LAUNCH_ID), + job_scope=None, owned_at=now, + job_name=f"Local\\BlenderSessiond.Setup.{LAUNCH_ID}", ) setup_owner._write_create_once(directory / "launch-receipt.json", receipt.to_dict()) monkeypatch.setattr(setup_owner, "process_matches", lambda *_args: False) - monkeypatch.setattr(setup_owner, "_OWNER_LOSS_WAIT_SECONDS", 0) - monkeypatch.setattr( - setup_owner, - "_job_exists", - lambda _name: (_ for _ in ()).throw(OSError("query failed")), + monkeypatch.setattr(setup_owner, "_STOP_WAIT_SECONDS", 0) + + recover = ( + setup_owner.status_setup + if operation == "status" + else setup_owner.stop_setup ) - result = setup_owner.status_setup( + result = recover( ATTEMPT_ID, expected_request_sha256=request.request_sha256, expected_launch_id=LAUNCH_ID, @@ -482,6 +550,78 @@ def test_owner_loss_job_query_error_is_cleanup_unverified( assert result.terminal.cleanup == "cleanup_unverified" +def test_owner_loss_process_query_error_remains_retryable( + tmp_path: Path, monkeypatch +) -> None: + now = datetime(2026, 9, 3, 12, tzinfo=UTC) + request = setup_owner.accept_launch_request( + raw_launch(now), state_root=tmp_path, now=now + ) + directory = tmp_path / "setup-attempts" / ATTEMPT_ID + receipt = setup_owner.LaunchReceipt( + attempt_id=ATTEMPT_ID, + launch_id=LAUNCH_ID, + request_sha256=request.request_sha256, + keeper_pid=10, + keeper_creation_time="windows:10", + root_pid=11, + root_creation_time="windows:11", + job_scope=setup_owner.JOB_SCOPE, + owned_at=now, + ) + setup_owner._write_create_once(directory / "launch-receipt.json", receipt.to_dict()) + monkeypatch.setattr(setup_owner, "_process_state", lambda *_args: "unknown") + monkeypatch.setattr(setup_owner, "_OWNER_LOSS_WAIT_SECONDS", 0) + result = setup_owner.status_setup( + ATTEMPT_ID, + expected_request_sha256=request.request_sha256, + expected_launch_id=LAUNCH_ID, + state_root=tmp_path, + platform_name="Windows", + ) + assert result.ownership_unverified + assert result.terminal is None + assert result.to_dict()["status"] == "ownership_unverified" + + +def test_owner_loss_reconciliation_returns_recovered_keeper( + tmp_path: Path, monkeypatch +) -> None: + now = datetime(2026, 9, 3, 12, tzinfo=UTC) + request = setup_owner.accept_launch_request( + raw_launch(now), state_root=tmp_path, now=now + ) + directory = tmp_path / "setup-attempts" / ATTEMPT_ID + receipt = setup_owner.LaunchReceipt( + attempt_id=ATTEMPT_ID, + launch_id=LAUNCH_ID, + request_sha256=request.request_sha256, + keeper_pid=10, + keeper_creation_time="windows:10", + root_pid=11, + root_creation_time="windows:11", + job_scope=setup_owner.JOB_SCOPE, + owned_at=now, + ) + setup_owner._write_create_once(directory / "launch-receipt.json", receipt.to_dict()) + keeper_states = iter(["absent", "matches"]) + + def process_state(pid: int, _identity: str) -> str: + return next(keeper_states) if pid == receipt.keeper_pid else "absent" + + monkeypatch.setattr(setup_owner, "_process_state", process_state) + monkeypatch.setattr(setup_owner, "_OWNER_LOSS_WAIT_SECONDS", 0) + result = setup_owner.status_setup( + ATTEMPT_ID, + expected_request_sha256=request.request_sha256, + expected_launch_id=LAUNCH_ID, + state_root=tmp_path, + platform_name="Windows", + ) + assert result.receipt == receipt + assert result.terminal is None + + def test_keeper_loss_reconciles_while_the_root_still_exists( tmp_path: Path, monkeypatch ) -> None: @@ -498,7 +638,7 @@ def test_keeper_loss_reconciles_while_the_root_still_exists( keeper_creation_time="windows:10", root_pid=11, root_creation_time="windows:11", - job_name=setup_owner._job_name(LAUNCH_ID), + job_scope=setup_owner.JOB_SCOPE, owned_at=now, ) setup_owner._write_create_once(directory / "launch-receipt.json", receipt.to_dict()) @@ -508,7 +648,6 @@ def test_keeper_loss_reconciles_while_the_root_still_exists( lambda pid, _identity: pid == receipt.root_pid, ) monkeypatch.setattr(setup_owner, "_OWNER_LOSS_WAIT_SECONDS", 0) - monkeypatch.setattr(setup_owner, "_job_exists", lambda _name: True) result = setup_owner.status_setup( ATTEMPT_ID, expected_request_sha256=request.request_sha256, @@ -520,7 +659,7 @@ def test_keeper_loss_reconciles_while_the_root_still_exists( assert result.terminal.cleanup == "cleanup_unverified" -def test_keeper_termination_error_is_cleanup_unverified( +def test_keeper_termination_error_remains_retryable( tmp_path: Path, monkeypatch ) -> None: now = datetime(2026, 9, 3, 12, tzinfo=UTC) @@ -536,11 +675,11 @@ def test_keeper_termination_error_is_cleanup_unverified( keeper_creation_time="windows:10", root_pid=11, root_creation_time="windows:11", - job_name=setup_owner._job_name(LAUNCH_ID), + job_scope=setup_owner.JOB_SCOPE, owned_at=now, ) setup_owner._write_create_once(directory / "launch-receipt.json", receipt.to_dict()) - monkeypatch.setattr(setup_owner, "process_matches", lambda *_args: True) + monkeypatch.setattr(setup_owner, "_process_state", lambda *_args: "matches") monkeypatch.setattr(setup_owner, "_STOP_WAIT_SECONDS", 0) from blendersessiond import windows_setup_process @@ -556,8 +695,8 @@ def test_keeper_termination_error_is_cleanup_unverified( state_root=tmp_path, platform_name="Windows", ) - assert result.terminal.outcome == "cleanup_unverified" - assert result.terminal.cleanup == "cleanup_unverified" + assert result.ownership_unverified + assert result.terminal is None def test_staged_script_read_is_bounded_to_the_declared_size( @@ -598,6 +737,55 @@ def open_spy(target: Path, *args, **kwargs): assert read_sizes == [len(SCRIPT) + 1] +def test_missing_staged_script_guard_uses_domain_error( + tmp_path: Path, monkeypatch +) -> None: + now = datetime(2026, 9, 3, 12, tzinfo=UTC) + request = setup_owner.accept_launch_request( + raw_launch(now), state_root=tmp_path, now=now + ) + + @contextmanager + def missing_guard(*_args, **_kwargs): + raise FileNotFoundError("missing") + yield + + monkeypatch.setattr(setup_owner, "_guard_setup_path", missing_guard) + with pytest.raises(setup_owner.SetupOwnerError, match="missing"): + setup_owner.read_staged_script(tmp_path, request) + + +@pytest.mark.parametrize( + "error", + [ + OSError("path query failed"), + pytest.param(None, id="authority-rejection"), + ], +) +def test_path_authority_rejection_uses_setup_owner_error( + error: OSError | None, + tmp_path: Path, + monkeypatch, + bypass_setup_path_authority, +) -> None: + from blendersessiond import windows_path_authority + + rejection = error or windows_path_authority.PathAuthorityError( + "untrusted authority" + ) + + @contextmanager + def rejected(*_args, **_kwargs): + raise rejection + yield + + monkeypatch.setattr(windows_path_authority, "guard_path", rejected) + monkeypatch.setattr(setup_owner.os, "name", "nt") + with pytest.raises(setup_owner.SetupOwnerError, match=str(rejection)): + with bypass_setup_path_authority(tmp_path): + pass + + @pytest.mark.parametrize( "mutate", [ @@ -736,5 +924,9 @@ def test_status_terminalizes_an_expired_dispatch_without_a_receipt( def test_non_windows_fails_closed(tmp_path: Path) -> None: with pytest.raises(setup_owner.SetupOwnerError, match="Windows"): setup_owner.status_setup( - ATTEMPT_ID, state_root=tmp_path, platform_name="Darwin" + ATTEMPT_ID, + expected_request_sha256="0" * 64, + expected_launch_id=LAUNCH_ID, + state_root=tmp_path, + platform_name="Darwin", ) diff --git a/tests/test_windows_path_authority.py b/tests/test_windows_path_authority.py new file mode 100644 index 0000000..c77b233 --- /dev/null +++ b/tests/test_windows_path_authority.py @@ -0,0 +1,261 @@ +from __future__ import annotations + +import csv +import ctypes +import os +import subprocess +from pathlib import PureWindowsPath + +import pytest + +from blendersessiond import windows_path_authority + + +class FakePathApi: + def __init__(self) -> None: + self.opened: list[str] = [] + self.closed: list[object] = [] + self.reparse: set[str] = set() + self.missing: set[str] = set() + self.security: dict[str, windows_path_authority.PathSecurity] = {} + + def open_path(self, path: str) -> str: + if path in self.missing: + raise FileNotFoundError(path) + self.opened.append(path) + return path + + def is_reparse_point(self, handle: str) -> bool: + return handle in self.reparse + + def read_security(self, handle: str) -> windows_path_authority.PathSecurity: + return self.security.get( + handle, + windows_path_authority.PathSecurity( + owner_sid=b"user", dacl_present=True, aces=() + ), + ) + + def trusted_sids(self) -> frozenset[bytes]: + return frozenset({b"user", b"system", b"administrators", b"creator"}) + + def close_handle(self, handle: object) -> None: + self.closed.append(handle) + + +class FakeAceApi: + def GetAce(self, dacl, index, output) -> bool: + assert index == 0 + ctypes.cast(output, ctypes.POINTER(ctypes.c_void_p)).contents.value = ( + dacl + ctypes.sizeof(windows_path_authority._Acl) + ) + return True + + +def test_guard_holds_every_ancestor_and_rejects_reparse_points() -> None: + api = FakePathApi() + api.reparse.add("C:\\trusted") + + with pytest.raises(windows_path_authority.PathAuthorityError, match="reparse"): + with windows_path_authority.guard_path( + PureWindowsPath(r"C:\trusted\state\attempt"), api=api + ): + pass + + assert api.opened == ["C:\\", "C:\\trusted"] + assert api.closed == list(reversed(api.opened)) + + +def test_guard_rejects_untrusted_write_access_on_authority_target() -> None: + api = FakePathApi() + api.security[r"C:\trusted\state"] = windows_path_authority.PathSecurity( + owner_sid=b"user", + dacl_present=True, + aces=( + windows_path_authority.AccessEntry( + sid=b"other", mask=windows_path_authority.FILE_WRITE_DATA + ), + ), + ) + + with pytest.raises(windows_path_authority.PathAuthorityError, match="untrusted"): + with windows_path_authority.guard_path( + PureWindowsPath(r"C:\trusted\state"), api=api + ): + pass + + +def test_guard_allows_untrusted_read_and_holds_handles_for_body() -> None: + api = FakePathApi() + api.security[r"C:\trusted\state"] = windows_path_authority.PathSecurity( + owner_sid=b"user", + dacl_present=True, + aces=(windows_path_authority.AccessEntry(sid=b"other", mask=0x1),), + ) + + with windows_path_authority.guard_path( + PureWindowsPath(r"C:\trusted\state"), api=api + ): + assert api.closed == [] + assert api.closed == list(reversed(api.opened)) + + +@pytest.mark.parametrize( + "security", + [ + windows_path_authority.PathSecurity( + owner_sid=b"other", dacl_present=True, aces=(), dacl_protected=True + ), + windows_path_authority.PathSecurity( + owner_sid=b"user", dacl_present=False, aces=(), dacl_protected=True + ), + windows_path_authority.PathSecurity( + owner_sid=b"user", dacl_present=True, aces=(), dacl_protected=False + ), + ], +) +def test_guard_requires_trusted_owner_and_present_dacl( + security: windows_path_authority.PathSecurity, +) -> None: + api = FakePathApi() + api.security[r"C:\trusted\state"] = security + with pytest.raises(windows_path_authority.PathAuthorityError): + with windows_path_authority.guard_path( + PureWindowsPath(r"C:\trusted\state"), api=api + ): + pass + + +def test_guard_can_secure_the_nearest_existing_parent_before_creation() -> None: + api = FakePathApi() + api.missing.add(r"C:\trusted\state\new") + api.missing.add(r"C:\trusted\state\new\attempt") + + with windows_path_authority.guard_path( + PureWindowsPath(r"C:\trusted\state\new\attempt"), + authority_root=PureWindowsPath(r"C:\trusted\state"), + allow_missing=True, + api=api, + ): + assert api.opened[-1] == r"C:\trusted\state" + + +def test_compound_access_allowed_ace_is_rejected() -> None: + size = ctypes.sizeof(windows_path_authority._Acl) + ctypes.sizeof( + windows_path_authority._AceHeader + ) + buffer = ctypes.create_string_buffer(size) + dacl = ctypes.addressof(buffer) + acl = ctypes.cast( + dacl, ctypes.POINTER(windows_path_authority._Acl) + ).contents + acl.ace_count = 1 + ace = ctypes.cast( + dacl + ctypes.sizeof(windows_path_authority._Acl), + ctypes.POINTER(windows_path_authority._AceHeader), + ).contents + ace.ace_type = 4 + ace.ace_size = ctypes.sizeof(windows_path_authority._AceHeader) + api = windows_path_authority._NativePathApi.__new__( + windows_path_authority._NativePathApi + ) + api.advapi = FakeAceApi() + + with pytest.raises( + windows_path_authority.PathAuthorityError, + match="unsupported access-allowed ACE", + ): + api._access_entries(dacl) + + +def _harden_for_current_user(path) -> None: + identity = subprocess.run( + ["whoami.exe", "/user", "/fo", "csv", "/nh"], + capture_output=True, + text=True, + check=True, + ) + user_sid = next(csv.reader([identity.stdout.strip()]))[1] + subprocess.run( + [ + "icacls.exe", + str(path), + "/inheritance:r", + "/grant:r", + f"*{user_sid}:(OI)(CI)F", + "*S-1-5-18:(OI)(CI)F", + "*S-1-5-32-544:(OI)(CI)F", + ], + capture_output=True, + check=True, + ) + + +@pytest.mark.skipif(os.name != "nt", reason="requires Windows path security") +def test_native_private_authority_root_is_accepted(tmp_path) -> None: + root = tmp_path / "private-authority" + root.mkdir() + _harden_for_current_user(root) + with windows_path_authority.guard_path(root): + pass + + +@pytest.mark.skipif(os.name != "nt", reason="requires Windows path security") +def test_native_inherited_untrusted_writer_is_rejected(tmp_path) -> None: + authority = tmp_path / "private-authority" + authority.mkdir() + _harden_for_current_user(authority) + parent = authority / "permissive-parent" + parent.mkdir() + subprocess.run( + [ + "icacls.exe", + str(parent), + "/grant", + "*S-1-1-0:(OI)(CI)M", + ], + capture_output=True, + check=True, + ) + child = parent / "inherited-child" + child.mkdir() + with pytest.raises(windows_path_authority.PathAuthorityError, match="untrusted"): + with windows_path_authority.guard_path(child, authority_root=authority): + pass + + +@pytest.mark.skipif(os.name != "nt", reason="requires Windows path security") +def test_native_inherit_only_untrusted_writer_is_rejected(tmp_path) -> None: + root = tmp_path / "inherit-only-root" + root.mkdir() + _harden_for_current_user(root) + subprocess.run( + [ + "icacls.exe", + str(root), + "/grant", + "*S-1-1-0:(OI)(CI)(IO)M", + ], + capture_output=True, + check=True, + ) + with pytest.raises(windows_path_authority.PathAuthorityError, match="untrusted"): + with windows_path_authority.guard_path(root): + pass + + +@pytest.mark.skipif(os.name != "nt", reason="requires Windows junctions") +def test_native_junction_ancestor_is_rejected(tmp_path) -> None: + target = tmp_path / "junction-target" + target.mkdir() + junction = tmp_path / "junction" + subprocess.run( + ["cmd.exe", "/d", "/c", "mklink", "/J", str(junction), str(target)], + capture_output=True, + check=True, + ) + with pytest.raises(windows_path_authority.PathAuthorityError, match="reparse"): + with windows_path_authority.guard_path( + junction / "attempt", allow_missing=True + ): + pass diff --git a/tests/test_windows_setup_process.py b/tests/test_windows_setup_process.py index 53fdedc..766da84 100644 --- a/tests/test_windows_setup_process.py +++ b/tests/test_windows_setup_process.py @@ -12,13 +12,15 @@ from blendersessiond import setup_owner, windows_setup_process +pytestmark = pytest.mark.usefixtures("bypass_setup_path_authority") + class FakeWin32: def __init__(self) -> None: self.calls: list[object] = [] - def create_job(self, name: str): - self.calls.append(("create_job", name)) + def create_job(self): + self.calls.append(("create_job", None)) return "job" def set_kill_on_close(self, job) -> None: @@ -62,8 +64,8 @@ def test_process_is_suspended_and_atomically_assigned_before_receipt( owned = windows_setup_process.WindowsSetupProcess.create_suspended( api=api, powershell_path=tmp_path / "powershell.exe", - job_name="Local\\BlenderSessiond.Setup.bbsl_test", ) + assert api.calls[0] == ("create_job", None) assert [call if isinstance(call, str) else call[0] for call in api.calls] == [ "create_job", "set_kill_on_close", @@ -91,7 +93,6 @@ def test_process_creation_rejects_a_child_outside_the_job(tmp_path: Path) -> Non windows_setup_process.WindowsSetupProcess.create_suspended( api=api, powershell_path=tmp_path / "powershell.exe", - job_name="Local\\BlenderSessiond.Setup.bbsl_test", ) closed = [ call[1] @@ -199,6 +200,42 @@ def test_concrete_windows_job_list_runtime_self_test() -> None: windows_setup_process.runtime_self_test() +def test_exact_process_state_keeps_access_failure_unknown(monkeypatch) -> None: + class Kernel: + def OpenProcess(self, *_args): + return 0 + + api = type("API", (), {"kernel": Kernel()})() + monkeypatch.setattr( + windows_setup_process.ctypes, + "get_last_error", + lambda: 5, + raising=False, + ) + assert windows_setup_process.exact_process_state(10, "windows:1", api=api) == ( + "unknown" + ) + + +def test_truncated_output_keeps_only_a_valid_utf8_prefix() -> None: + content = "snowman ☃".encode()[:-1] + assert windows_setup_process._valid_utf8_prefix( + content, truncated=True + ) == b"snowman " + + +def test_truncated_output_drops_bytes_after_first_invalid_sequence() -> None: + content = b"valid\xffalso-valid" + assert windows_setup_process._valid_utf8_prefix( + content, truncated=True + ) == b"valid" + + +def test_untruncated_invalid_output_fails_closed() -> None: + with pytest.raises(UnicodeDecodeError): + windows_setup_process._valid_utf8_prefix(b"valid\xff", truncated=False) + + def _attempt(tmp_path: Path, deadline: datetime) -> Path: attempt_id = "bbsa_" + "A" * 43 launch_id = "bbsl_" + "B" * 43 @@ -334,7 +371,7 @@ def resume() -> None: raise OSError("stop after observation") owned.resume = resume - monkeypatch.setattr(windows_setup_process, "file_lock", tracked_lock) + monkeypatch.setattr(setup_owner, "_setup_lock", tracked_lock) monkeypatch.setattr( windows_setup_process.WindowsSetupProcess, "create_suspended", @@ -427,3 +464,43 @@ def now(cls, _tz=None) -> datetime: ) assert outcome == "timed_out" assert terminated == [] + + +def test_descendant_wait_uses_a_monotonic_deadline( + tmp_path: Path, monkeypatch +) -> None: + wall_deadline = datetime(2026, 9, 3, 12, tzinfo=UTC) + terminated: list[bool] = [] + owned = type( + "Owned", + (), + { + "empty": lambda _self: False, + "terminate": lambda _self: terminated.append(True), + }, + )() + + class Clock: + @classmethod + def now(cls, _tz=None) -> datetime: + return wall_deadline - timedelta(days=1) + + monkeypatch.setattr(windows_setup_process, "datetime", Clock) + monkeypatch.setattr(windows_setup_process.time, "monotonic", lambda: 11.0) + outcome = windows_setup_process._wait_for_owned_job( + owned, + wall_deadline, + tmp_path / ".stop", + monotonic_deadline=10.0, + ) + assert outcome == "timed_out" + assert terminated == [True] + + +def test_truncated_output_keeps_only_valid_utf8() -> None: + content = b"a" * (windows_setup_process._MAX_STREAM_BYTES - 1) + "€".encode() + bounded = windows_setup_process._valid_utf8_prefix( + content[: windows_setup_process._MAX_STREAM_BYTES], + truncated=True, + ) + assert bounded == b"a" * (windows_setup_process._MAX_STREAM_BYTES - 1)