Repository navigation
fix(setup-owner): close merged ownership gaps - #32
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
accept_launch_request() now guards path authority before creating directories, which can make first-run setup-owner launch fail when the default state root doesn’t already exist on Windows.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR tightens the Windows “setup-owner” security and lifecycle contract by removing reliance on a re-openable named Job Object, adding path-authority validation for staged state/script paths, and making transient “can’t verify ownership/cleanup” conditions retryable instead of terminal.
Changes:
- Switch setup ownership to an unnamed keeper-only kill-on-close Job and add retryable
ownership_unverifiedstatus when process access/query is unknown. - Add Windows path authority enforcement (anti-reparse + owner/DACL validation) and apply it broadly to attempt state markers, JSON records, keeper dispatch, and staged script reads.
- Harden parsing/IO semantics: strict JSON (no dup keys/BOM/non-finite), bounded output with UTF-8-prefix truncation, monotonic deadline support, and updated CLI exit codes.
File summaries
| File | Description |
|---|---|
| tests/test_windows_setup_process.py | Updates setup-process tests for unnamed Job creation, retryable unknown process state, UTF-8 truncation, and monotonic deadlines. |
| tests/test_windows_path_authority.py | Adds unit + native Windows tests for path-authority guarding (reparse, DACL/owner checks, allow-missing behavior). |
| tests/test_setup_owner.py | Expands setup-owner regression coverage for strict JSON, fence validation, retryable ownership loss, legacy named-job receipt recovery, and path guard error shaping. |
| tests/test_cli.py | Validates setup-owner subcommand exit status behavior for terminal vs unverified states. |
| src/blendersessiond/windows_setup_process.py | Implements unnamed kill-on-close Job usage, exact_process_state, UTF-8-prefix truncation, monotonic deadlines, and guarded/locked keeper execution. |
| src/blendersessiond/windows_path_authority.py | New module enforcing Windows path authority via handle-based traversal, reparse detection, and DACL/owner validation. |
| src/blendersessiond/setup_owner.py | Integrates strict JSON, retryable ownership semantics, legacy named-job compatibility, and path-authority + locking helpers for all attempt state I/O. |
| src/blendersessiond/cli.py | Adjusts setup-owner command exit codes based on terminal success/cleanup proof and ownership_unverified. |
| docs/adr/0005-fenced-windows-setup-owner.md | Updates ADR to reflect unnamed Job behavior, retryable ownership loss, and path authority validation requirements. |
Review details
- Files reviewed: 9/9 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Independent merge gate passed on
|
The merged Windows setup owner still left concrete gaps for Blender Box issue #12: a same-user child could reopen the named Job Object, process-query failures could become durable cleanup claims, staged setup state could be redirected or rewritten through permissive Windows path authority, and legacy session-local Job absence could be mistaken for global tree cleanup.
This follow-up switches new attempts to an unnamed keeper-only kill-on-close Job, keeps legacy named-Job receipts readable and fenced without trusting name absence as cleanup proof, makes unknown ownership explicitly retryable, requires complete request and Launch fences, and validates the full setup path chain by handle against reparse points, owner changes, unsafe DACLs, and unsupported compound allow ACEs. It also tightens strict JSON, launch-failure persistence, UTF-8 output truncation, runtime capability proof, and terminal CLI exit semantics. The ADR and focused regression coverage describe the upgraded contract.
Verification:
uv run ruff check .uv run pytest -m "not e2e" -q— 183 passed, 5 skipped, 22 deselecteduv run pytest -m "e2e and not real_blender" -q— 18 passed, 192 deselectedRelated to BramVR/blender-box#12. This PR does not close that issue; Blender Box integration remains gated on this dependency landing.
Model: GPT-5.6 Sol (high)
Harness: Codex desktop with autoreview and the repository-owned Windows verification workflow.