Skip to content

fix(setup-owner): close merged ownership gaps - #32

Merged
BramVR merged 4 commits into
mainfrom
codex/windows-setup-owner-hardening
Sep 3, 2026
Merged

BramVR merged 4 commits into
mainfrom
codex/windows-setup-owner-hardening

Conversation

@BramVR

@BramVR BramVR commented Sep 3, 2026 •

Copy link
Copy Markdown
Owner

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 deselected
  • uv run pytest -m "e2e and not real_blender" -q — 18 passed, 192 deselected
  • autoreview — clean, no actionable findings
  • no-comments audit — 0 new comments/docstrings, 0 deletions, no MUST KILL findings
  • exact-wheel proof on the authorized owned Windows host — capability and poisoned-SystemRoot checks, reparse and permissive-DACL rejection, replay recovery, stale-stop fencing, owned root/child exit, unrelated decoy survival, repeated stop, honest owner-loss reporting, successful completion, and verified scratch cleanup; Blender was not launched

Related 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.

Copilot AI lite review requested due to automatic review settings September 3, 2026 18:27

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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_unverified status 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.

Comment thread src/blendersessiond/setup_owner.py
@BramVR

BramVR commented Sep 3, 2026

Copy link
Copy Markdown
Owner Author

Independent merge gate passed on d362ed6f1da44d971506823c003914358f38cd4a.

  • Terra 5.6 high re-reviewed the complete follow-up and the final Windows-specific corrections.
  • Local proof: Ruff passed; 183 unit/integration tests passed with 5 platform skips; 18 fake-boundary E2E tests passed.
  • Exact-wheel Windows proof passed with hostile environment, path/ACL rejection, replay, fencing, tree termination, decoy survival, retryable ownership uncertainty, and clean completion.
  • Both Windows CI matrices, both macOS and Ubuntu matrices, required real-Blender Ubuntu smoke, and best-effort macOS smoke passed on the exact head.
  • The missing state-root review comment was traced and is intentional fail-closed behavior. ADR 0005 assigns private-DACL root provisioning to Blender Box before staging.
  • No accepted or actionable findings remain.

@BramVR
BramVR merged commit a972992 into main Sep 3, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants