aiorellana502 · Oct 14, 2026
diff --git a/lib/review-candidate-view-owner.ts b/lib/review-candidate-view-owner.ts
@@ -208,19 +208,19 @@ function enforcePrivateWindowsDacl(path: string, identity: WindowsAclIdentity =
 	assertPrivateWindowsDacl(path, "directory", true, identity);
 }
 
-function privateWindowsDacl(path: string, kind: WindowsObjectKind, protectedDacl: boolean, enforce = false): void {
+function privateWindowsDacl(path: string, kind: WindowsObjectKind, protectedDacl: boolean, enforce = false, windowsIdentity?: WindowsAclIdentity): void {
 	if (testingWindowsAclAuthority !== undefined) return testingWindowsAclAuthority(path);
-	if (enforce) enforcePrivateWindowsDacl(path);
-	else assertPrivateWindowsDacl(path, kind, protectedDacl);
+	if (enforce) enforcePrivateWindowsDacl(path, windowsIdentity);
+	else assertPrivateWindowsDacl(path, kind, protectedDacl, windowsIdentity);
 }
 
-function privateWindowsCandidateOwnerBoundary(commonDir: string, enforce = false): void {
+function privateWindowsCandidateOwnerBoundary(commonDir: string, enforce = false, windowsIdentity?: WindowsAclIdentity): void {
 	const boundary = [commonDir, join(commonDir, "gentle-ai"), join(commonDir, "gentle-ai", "candidate-views")];
 	if (testingWindowsAclAuthority !== undefined) {
 		for (const path of boundary) testingWindowsAclAuthority(path);
 		return;
 	}
-	const identity = windowsAclIdentity();
+	const identity = windowsIdentity ?? windowsAclIdentity();
 	if (enforce) {
 		for (const path of boundary) validatePrivateWindowsOwner(windowsOwnerSid(path, "directory"), identity.user);
 		for (const path of boundary) enforcePrivateWindowsDacl(path, identity);
@@ -304,16 +304,23 @@ function directory(path: string, privateMode = false, platform: NodeJS.Platform
 	return `${stat.dev}:${stat.ino}`;
 }
 
-export function assertCandidateOwnerParent(commonDir: string, platform: NodeJS.Platform = process.platform): string {
+function checkedCandidateOwnerParent(commonDir: string, platform: NodeJS.Platform): { parent: string; windowsIdentity?: WindowsAclIdentity } {
 	directory(commonDir, false, platform);
 	const control = join(commonDir, "gentle-ai");
 	directory(control, false, platform);
 	const parent = join(commonDir, "gentle-ai", "candidate-views");
 	if (platform === "win32") {
 		directory(parent, false, platform);
-		privateWindowsCandidateOwnerBoundary(commonDir);
-	} else directory(parent, true, platform, true);
-	return parent;
+		const windowsIdentity = testingWindowsAclAuthority === undefined ? windowsAclIdentity() : undefined;
+		privateWindowsCandidateOwnerBoundary(commonDir, false, windowsIdentity);
+		return { parent, windowsIdentity };
+	}
+	directory(parent, true, platform, true);
+	return { parent };
+}
+
+export function assertCandidateOwnerParent(commonDir: string, platform: NodeJS.Platform = process.platform): string {
+	return checkedCandidateOwnerParent(commonDir, platform).parent;
 }
 
 export function prepareCandidateOwnerParent(commonDir: string, platform: NodeJS.Platform = process.platform): string {
@@ -329,13 +336,13 @@ export function prepareCandidateOwnerParent(commonDir: string, platform: NodeJS.
 	return assertCandidateOwnerParent(commonDir, platform);
 }
 
-function regular(path: string, privateMode = false, platform: NodeJS.Platform = process.platform): string {
+function regular(path: string, privateMode = false, platform: NodeJS.Platform = process.platform, windowsIdentity?: WindowsAclIdentity): string {
 	// Keep 64-bit identity exact without a second stat lookup racing the privacy checks.
 	const stat = lstatSync(path, { bigint: true });
 	const uid = process.getuid?.();
 	if (!stat.isFile() || stat.isSymbolicLink() || stat.nlink !== 1n || !samePath(realpathSync(path), path, platform) || stat.size > 16384n ||
 		(privateMode && platform !== "win32" && (uid === undefined || stat.uid !== BigInt(uid) || (stat.mode & 0o777n) !== 0o600n))) throw new Error("Unsafe candidate owner file");
-	if (privateMode && platform === "win32") privateWindowsDacl(path, "file", false);
+	if (privateMode && platform === "win32") privateWindowsDacl(path, "file", false, false, windowsIdentity);
 	return `${stat.dev}:${stat.ino}`;
 }
 
@@ -347,47 +354,50 @@ function syncDirectory(path: string): void {
 	try { fsyncSync(fd); } finally { closeSync(fd); }
 }
 
-function removeExactPrivateFile(path: string, identity: string, platform: NodeJS.Platform): void {
+function removeExactPrivateFile(path: string, identity: string, platform: NodeJS.Platform, windowsIdentity?: WindowsAclIdentity): void {
 	try {
-		if (regular(path, true, platform) !== identity) return;
+		if (regular(path, true, platform, windowsIdentity) !== identity) return;
 		unlinkSync(path);
 		syncDirectory(dirname(path));
 	} catch { /* An unproven or replaced file survives. */ }
 }
 
-function exclusiveFile(path: string, content: string, platform: NodeJS.Platform = process.platform, rollbackOnFailure = false): string {
+function exclusiveFile(path: string, content: string, platform: NodeJS.Platform = process.platform, rollbackOnFailure = false, windowsIdentity?: WindowsAclIdentity): string {
 	let identity: string | undefined;
 	try {
 		const fd = openSync(path, "wx", 0o600);
 		try {
-			identity = regular(path, true, platform);
+			identity = regular(path, true, platform, windowsIdentity);
 			writeFileSync(fd, content);
 			fsyncSync(fd);
 		} finally { closeSync(fd); }
 		syncDirectory(dirname(path));
 		return identity;
 	} catch (error) {
-		if (rollbackOnFailure && identity !== undefined) removeExactPrivateFile(path, identity, platform);
+		if (rollbackOnFailure && identity !== undefined) removeExactPrivateFile(path, identity, platform, windowsIdentity);
 		throw error;
 	}
 }
 
 function markerPath(root: string): string { return `${root}.owner.json`; }
 
 export function createCandidateOwner(commonDir: string, root: string, platform: NodeJS.Platform = process.platform): CandidateViewOwner {
-	const parent = assertCandidateOwnerParent(commonDir, platform);
+	// Reuse only identity within this module-controlled synchronous creation.
+	// Owner/DACL observations stay fresh, including rollback. Nothing is retained
+	// across calls or passed into callback-based removal/sweeping operations.
+	const { parent, windowsIdentity } = checkedCandidateOwnerParent(commonDir, platform);
 	const uuid = basename(root);
 	if (!UUID.test(uuid) || root !== join(parent, uuid) || lstatSync(root, { throwIfNoEntry: false })) throw new Error("Unsafe candidate owner root");
 	const owner: CandidateViewOwner = { version: 1, uuid, token: randomUUID(), pid: process.pid, host: localHost(), root, commonDir };
 	// Any write/fsync failure aborts creation BEFORE Git can register the view.
 	const marker = markerPath(root);
 	let markerIdentity: string | undefined;
 	try {
-		markerIdentity = exclusiveFile(marker, JSON.stringify(owner), platform, true);
+		markerIdentity = exclusiveFile(marker, JSON.stringify(owner), platform, true, windowsIdentity);
 		syncDirectory(dirname(parent));
 		syncDirectory(commonDir);
 	} catch (error) {
-		if (markerIdentity !== undefined) removeExactPrivateFile(marker, markerIdentity, platform);
+		if (markerIdentity !== undefined) removeExactPrivateFile(marker, markerIdentity, platform, windowsIdentity);
 		throw error;
 	}
 	return Object.freeze(owner);

diff --git a/odd/tasks/issue-2074-windows-identity-reuse.md b/odd/tasks/issue-2074-windows-identity-reuse.md
@@ -0,0 +1,22 @@
+# Issue #2074 — Windows owner creation repeats SID identity probes
+
+`createCandidateOwner` acquires Windows user/administrator SID identity twice within one synchronous creation (parent-boundary validation, then marker private-file validation). Reuse the identity acquired for parent validation through the rest of that same creation, including rollback, without caching it across operations.
+
+## Specs
+
+- S1: "acquire Windows user/administrator identity once within one module-controlled synchronous owner creation, reuse only that identity through creation-local validation/rollback, and reacquire identity on the next operation."
+- S2: "do not cache filesystem owner or DACL facts, add a process-wide SID cache, weaken private-owner checks, change public signatures, or pass retained identity into callback-based cleanup/sweeping operations."
+- S3: "Preparation already acquires once and must remain unchanged." (`prepareCandidateOwnerParent` / `assertCandidateOwnerParent` keep acquiring identity exactly once per call, as today.)
+- S4: "Identity failures must fail closed, and rollback must preserve a marker whose current owner cannot be re-proven."
+- S5: Reference implementation to study and adapt: baseline commit `2d7ab4ba3ad09b652d0fa1de56842e02b5db4669`, preserved candidate commit `1efccb5a01445d84ca554e0dea9f733362c4a8c8` (extracted out of PR #2068 to keep that PR scoped to an unrelated subagent Git-cache issue). Candidate reported: focused checks 12/12 no skips, typecheck zero diagnostics, independent security verification PASS, Windows bounded sample mean 986.2ms baseline vs 817.9ms candidate (~17.1%), user/administrator SID probes 2→1 per creation, owner/DACL observations unchanged at 4 each.
+
+## Tasks
+
+- [x] T1 (S1-S5): Adapt the preserved candidate diff onto current `main` — `lib/review-candidate-view-owner.ts`, `tests/review-owner-identity.test.ts`. Candidate diff applied byte-identical, no rebase conflicts.
+- [x] T2 (S1-S4): Run focused tests + typecheck locally on Windows; confirm fail-closed/rollback behavior. `node --test tests/review-owner-identity.test.ts`: 5/5 pass. `pnpm run typecheck`: 0 diagnostics.
+- [x] T3 (risk item 2 — security/identity): Independent `gentle-ai-verify` pass before closing. PASS on S1-S5 with file:line evidence; 3 extra regression tests it proposed were added to `tests/review-owner-identity.test.ts` (8/8 pass). One pre-existing, unrelated advisory: `tests/review-candidate-view.test.ts` times out identically on baseline and candidate (environment issue, not caused by this change).
+- [ ] T4: Open PR against `Gentleman-Programming/gentle-shell`, `Closes #2074`.
+
+## Log
+
+- L1 (verbatim issue body): "On Windows, one module-controlled synchronous `createCandidateOwner` operation acquires the same user/local-administrator SID identity twice: once for parent-boundary validation and again for the new owner marker's private-file validation. The identity subprocesses add avoidable latency even though the parent and marker checks belong to the same synchronous creation. [...] This is distinct from #990 and #1446 (probe timeouts), #1023 (trusted-helper resolution), #775 (Windows permission validation) and #1809 (identity-precision fixture failure). The proposed identity lifetime remains restricted to creation; preparation already acquires identity once and is unchanged."
diff --git a/tests/review-owner-identity.test.ts b/tests/review-owner-identity.test.ts
@@ -0,0 +1,175 @@
+import assert from "node:assert/strict";
+import childProcess from "node:child_process";
+import { randomUUID } from "node:crypto";
+import fs from "node:fs";
+import { syncBuiltinESMExports } from "node:module";
+import { dirname, join, resolve } from "node:path";
+import test from "node:test";
+import {
+	assertCandidateOwnerParent,
+	createCandidateOwner,
+	prepareCandidateOwnerParent,
+	sweepCandidateOwners,
+} from "../lib/review-candidate-view-owner.ts";
+
+function fixture(t: test.TestContext) {
+	fs.mkdirSync(resolve("tmp"), { recursive: true });
+	const base = fs.realpathSync(fs.mkdtempSync(join(resolve("tmp"), "owner-identity-")));
+	const commonDir = join(base, "common");
+	const parent = join(commonDir, "gentle-ai", "candidate-views");
+	fs.mkdirSync(parent, { recursive: true });
+	const boundary = [commonDir, dirname(parent), parent];
+	let user = "S-1-5-21-1-2-3-1001";
+	let identityFailure = "";
+	let replacedMarker = false;
+	const probes = { user: 0, administrator: 0, owner: [] as string[], dacl: [] as string[], enforce: [] as string[] };
+	const native = fs.realpathSync.native;
+	t.mock.method(fs.realpathSync, "native", (path: string) =>
+		path.startsWith("\\\\?\\GLOBALROOT\\SystemRoot\\System32") ? path : native(path));
+	t.mock.method(childProcess, "execFileSync", (command: string, args: string[], options: any) => {
+		const name = command.split(/[\\/]/).at(-1);
+		if (name === "whoami.exe") {
+			probes.user++;
+			return identityFailure === "user" ? "unavailable" : user;
+		}
+		if (name === "icacls.exe") {
+			probes.dacl.push(args[0]);
+			const directory = fs.lstatSync(args[0]).isDirectory();
+			const flags = directory ? "OICI" : "ID";
+			const dacl = `D:${directory ? "P" : ""}(A;${flags};FA;;;${user})(A;${flags};FA;;;SY)(A;${flags};FA;;;BA)`;
+			fs.writeFileSync(join(options.cwd, args[2]), dacl, "utf16le");
+			return "";
+		}
+		if (name === "powershell.exe") {
+			const script = Buffer.from(args.at(-1)!, "base64").toString("utf16le");
+			if (script.includes("RawSecurityDescriptor")) {
+				probes.administrator++;
+				return identityFailure === "administrator" ? "unavailable" : "S-1-5-21-1-2-3-500";
+			}
+			if (script.includes("SetAccessControl")) {
+				probes.enforce.push(options.env.GENTLE_PI_CANDIDATE_ACL_PATH);
+				return "";
+			}
+			const path = options.env.GENTLE_PI_CANDIDATE_OWNER_PATH;
+			probes.owner.push(path);
+			return replacedMarker && path.endsWith(".owner.json") ? "S-1-5-21-9-8-7-1002" : user;
+		}
+		throw new Error(`Unexpected subprocess: ${name}`);
+	});
+	syncBuiltinESMExports();
+	t.after(() => {
+		t.mock.restoreAll();
+		syncBuiltinESMExports();
+		fs.rmSync(base, { recursive: true, force: true });
+	});
+	return { commonDir, parent, boundary, probes,
+		changeUser: () => { user = "S-1-5-21-4-5-6-1002"; },
+		failIdentity: (kind: string) => { identityFailure = kind; },
+		replaceMarker: () => { replacedMarker = true; } };
+}
+
+test("Windows creation reuses identity only within one operation and preserves every filesystem probe", (t) => {
+	const f = fixture(t);
+	for (let operation = 1; operation <= 2; operation++) {
+		if (operation === 2) f.changeUser();
+		const root = join(f.parent, randomUUID());
+		const owner = createCandidateOwner(f.commonDir, root, "win32");
+		const marker = `${root}.owner.json`;
+		assert.equal(owner.root, root);
+		assert.deepEqual(JSON.parse(fs.readFileSync(marker, "utf8")), owner);
+		assert.equal(fs.existsSync(root), false);
+		assert.equal(f.probes.user, operation, "one fresh user SID per creation");
+		assert.equal(f.probes.administrator, operation, "one fresh administrator SID per creation");
+		assert.deepEqual(f.probes.owner.slice((operation - 1) * 4), [...f.boundary, marker]);
+		assert.deepEqual(f.probes.dacl.slice((operation - 1) * 4), [...f.boundary, marker]);
+		assert.deepEqual(f.probes.enforce, []);
+	}
+});
+
+test("Windows preparation retains fresh pre-write and post-write owner/DACL checks", (t) => {
+	const f = fixture(t);
+	assert.equal(prepareCandidateOwnerParent(f.commonDir, "win32"), f.parent);
+	assert.equal(f.probes.user, 1);
+	assert.equal(f.probes.administrator, 1);
+	assert.deepEqual(f.probes.owner, [...f.boundary, ...f.boundary.flatMap(path => [path, path])]);
+	assert.deepEqual(f.probes.dacl, f.boundary);
+	assert.deepEqual(f.probes.enforce, f.boundary);
+});
+
+for (const kind of ["user", "administrator"]) test(`Windows creation fails closed on fresh ${kind} identity failure`, (t) => {
+	const f = fixture(t);
+	const first = createCandidateOwner(f.commonDir, join(f.parent, randomUUID()), "win32");
+	const marker = `${first.root}.owner.json`;
+	const before = fs.readFileSync(marker, "utf8");
+	const stored = fs.readdirSync(f.parent);
+	f.failIdentity(kind);
+	const root = join(f.parent, randomUUID());
+	assert.throws(() => createCandidateOwner(f.commonDir, root, "win32"), /SID is unavailable/);
+	assert.equal(fs.existsSync(root), false);
+	assert.equal(fs.existsSync(`${root}.owner.json`), false);
+	assert.equal(fs.readFileSync(marker, "utf8"), before);
+	assert.deepEqual(fs.readdirSync(f.parent), stored);
+	assert.equal(f.probes.owner.length, 4);
+	assert.equal(f.probes.dacl.length, 4);
+});
+
+test("Windows creation rollback rechecks a replaced marker owner before removal", (t) => {
+	const f = fixture(t);
+	const write = fs.writeFileSync;
+	const root = join(f.parent, randomUUID());
+	const marker = `${root}.owner.json`;
+	t.mock.method(fs, "writeFileSync", (...args: any[]) => {
+		if (typeof args[0] === "number") {
+			f.replaceMarker();
+			throw new Error("fixture write failure");
+		}
+		return (write as any)(...args);
+	});
+	syncBuiltinESMExports();
+	assert.throws(() => createCandidateOwner(f.commonDir, root, "win32"), /fixture write failure/);
+	assert.equal(f.probes.user, 1);
+	assert.equal(f.probes.administrator, 1);
+	assert.deepEqual(f.probes.owner, [...f.boundary, marker, marker]);
+	assert.deepEqual(f.probes.dacl, [...f.boundary, marker]);
+	assert.equal(fs.existsSync(marker), true, "unproven marker survives rollback");
+	assert.equal(fs.readFileSync(marker, "utf8"), "");
+	assert.equal(fs.existsSync(root), false);
+});
+
+test("Windows parent assertion acquires fresh identity exactly once per call", (t) => {
+	const f = fixture(t);
+	for (let call = 1; call <= 2; call++) {
+		assert.equal(assertCandidateOwnerParent(f.commonDir, "win32"), f.parent);
+		assert.equal(f.probes.user, call);
+		assert.equal(f.probes.administrator, call);
+	}
+	assert.deepEqual(f.probes.owner, [...f.boundary, ...f.boundary]);
+	assert.deepEqual(f.probes.dacl, [...f.boundary, ...f.boundary]);
+});
+
+test("Windows creation rollback removes a still-proven marker with one identity", (t) => {
+	const f = fixture(t);
+	const write = fs.writeFileSync;
+	const root = join(f.parent, randomUUID());
+	const marker = `${root}.owner.json`;
+	t.mock.method(fs, "writeFileSync", (...args: any[]) => {
+		if (typeof args[0] === "number") throw new Error("fixture write failure");
+		return (write as any)(...args);
+	});
+	syncBuiltinESMExports();
+	assert.throws(() => createCandidateOwner(f.commonDir, root, "win32"), /fixture write failure/);
+	assert.equal(f.probes.user, 1);
+	assert.equal(f.probes.administrator, 1);
+	assert.deepEqual(f.probes.owner, [...f.boundary, marker, marker]);
+	assert.deepEqual(f.probes.dacl, [...f.boundary, marker, marker]);
+	assert.equal(fs.existsSync(marker), false);
+	assert.deepEqual(fs.readdirSync(f.parent), []);
+});
+
+test("Windows sweep after creation reacquires identity for every check", (t) => {
+	const f = fixture(t);
+	createCandidateOwner(f.commonDir, join(f.parent, randomUUID()), "win32");
+	sweepCandidateOwners(f.commonDir, () => { throw new Error("git"); }, () => {}, "win32");
+	assert.equal(f.probes.user, 4, "1 creation + 3 independent sweep acquisitions");
+	assert.equal(f.probes.administrator, 4);
+});