perf(review): reuse Windows identity within owner creation

[aiorellana502]

Closes #2074

PR Type

• Bug fix / performance fix

Summary

• createCandidateOwner acquired the Windows user/local-administrator SID identity twice within one synchronous creation (parent-boundary check, then the new owner marker's private-file check).
• The identity acquired for the parent check is now threaded through the rest of that same creation, including rollback, so it is fetched once per creation instead of twice.
• Preparation (prepareCandidateOwnerParent), cleanup, and sweep operations are unchanged and still acquire identity independently — nothing is cached across separate operations.

Changes Table

File	Change
lib/review-candidate-view-owner.ts	checkedCandidateOwnerParent (new) replaces the body of assertCandidateOwnerParent, returning the already-acquired identity; createCandidateOwner threads it through marker validation and rollback. privateWindowsDacl / privateWindowsCandidateOwnerBoundary accept an optional identity param instead of always re-fetching.
tests/review-owner-identity.test.ts	New: 8 focused tests mocking whoami.exe/icacls.exe/powershell.exe via childProcess.execFileSync, covering single-acquisition reuse, fail-closed identity errors, and rollback re-proving a replaced marker's owner before deciding whether to remove it.
odd/tasks/issue-2074-windows-identity-reuse.md	ODD tracking doc for this fix (specs, tasks, log).

Test Plan

• node --test tests/review-owner-identity.test.ts: 8/8 pass
• pnpm run typecheck: 0 diagnostics
• Independent review (separate agent pass, not part of this PR's checks) confirmed: single identity acquisition per creation, no process-wide cache added, no owner/DACL filesystem fact skipped, public exported signatures unchanged, identity failures still fail closed, rollback still re-validates the marker's current owner before removing it.

Contributor Checklist

• Linked an approved issue (status:approved on perf(review): Windows owner creation repeats SID identity probes #2074)
• Conventional commit format
• No Co-Authored-By trailers
• Scope restricted to the identity-reuse change described in perf(review): Windows owner creation repeats SID identity probes #2074; preparation/cleanup/sweep behavior untouched

Summary by CodeRabbit

• Bug Fixes
  • Windows file ownership and access checks now use a consistent identity throughout candidate creation, reducing inconsistent validation.
  • Checks still reacquire identity between separate operations and fail safely when identity information is unavailable. marker-file changes are also checked during rollback.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository UI
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 512c271f-2cb7-4dde-bb70-f750d2ffd0e7

📥 Commits

Reviewing files that changed from the base of the PR and between f69cf2b and 23b5bbf.

📒 Files selected for processing: 3

• lib/review-candidate-view-owner.ts
• odd/tasks/issue-2074-windows-identity-reuse.md
• tests/review-owner-identity.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

📝 Walkthrough

Windows candidate-owner creation now reuses the identity acquired during parent validation for marker-file checks and rollback. Added tests cover identity acquisition within and across operations, lookup failures, owner and DACL checks, and rollback behavior.

Changes: Windows owner identity

Layer / File(s)	Summary
Pass identity through owner creation lib/review-candidate-view-owner.ts	Parent validation returns the Windows identity used for boundary checks. createCandidateOwner reuses it for marker validation and failure rollback. Private-file checks accept the optional identity.
Test identity scope and rollback tests/review-owner-identity.test.ts, odd/tasks/issue-2074-windows-identity-reuse.md	Tests check identity acquisition counts, owner and DACL observations, lookup failures, and rollback cases. The issue document records the implementation and verification checklist.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 23b5b

This change reduces redundant Windows identity lookups during owner creation while preserving fail-closed behavior and rollback checks. No concrete merge-blocking risk was found.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 2 files. (1 skipped: 1…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: reusing the Windows identity during candidate-owner creation.
Linked Issues check		Issue #2074 requires one Windows user/local-administrator SID acquisition per synchronous createCandidateOwner, reuse only within that creation, fresh filesystem owner/DACL checks, independent prepa…
Out of Scope Changes check		The changes stay within issue #2074. The source changes are limited to creation-local Windows identity threading. The new tests verify the required security and acquisition boundaries. The ODD documen…

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 2 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[aiorellana502]

Closing as duplicate: issue #2074 was resolved by #2262 (merged as 692a4dd), which landed the same fix first and closed the issue. Thanks for the parallel effort!