diff --git a/src/lib/windows-secret-acl.ts b/src/lib/windows-secret-acl.ts index 04ecb25ea0..120b8fd526 100644 --- a/src/lib/windows-secret-acl.ts +++ b/src/lib/windows-secret-acl.ts @@ -33,7 +33,6 @@ import { existsSync, statSync } from "node:fs"; import { env, platform } from "node:process"; import { resolveTrustedWindowsIcaclsExe } from "./windows-elevation"; import { - cachedCurrentWindowsIdentity, resolveCurrentWindowsPrincipal, resolveCurrentWindowsPrincipalAsync, setSyntheticWindowsPrincipalForTests, @@ -501,69 +500,6 @@ function grantAce(user: string, directory: boolean): string { return directory ? `${user}:(OI)(CI)(F)` : `${user}:(F)`; } -function existingAclIsCompliant( - targetPath: string, - directory: boolean, - stdout: string, - ownerName: string, -): boolean { - const lines = stdout.replaceAll("\r", "").split("\n"); - const first = lines.shift(); - if (!first || first.slice(0, targetPath.length).toLowerCase() !== targetPath.toLowerCase()) { - return false; - } - const separator = first[targetPath.length]; - if (separator !== undefined && !/\s/.test(separator)) return false; - - const aceLines: string[] = []; - const firstAce = first.slice(targetPath.length).trim(); - if (firstAce) aceLines.push(firstAce); - for (const line of lines) { - if (!line.trim()) break; - // Localized summary text is not indented like a continuation ACE. - if (!/^\s/.test(line)) break; - aceLines.push(line.trim()); - } - if (aceLines.length !== 1) return false; - - const match = /^([^:]+):((?:\([A-Z]+\))+)$/.exec(aceLines[0]!); - if (!match || match[1]!.trim().toLowerCase() !== ownerName.toLowerCase()) return false; - const rights = [...match[2]!.matchAll(/\(([A-Z]+)\)/g)].map(part => part[1]); - const expected = directory ? ["OI", "CI", "F"] : ["F"]; - return rights.length === expected.length && rights.every((right, index) => right === expected[index]); -} - -function shouldVerifyExistingAcl(): boolean { - return env["OPENCODEX_ACL_VERIFY_EXISTING"] === "1"; -} - -function existingAclAlreadyCompliant(targetPath: string, directory: boolean): boolean { - if (!shouldVerifyExistingAcl()) return false; - const identity = cachedCurrentWindowsIdentity(); - if (!identity) return false; - try { - const result = icaclsRunner([targetPath], resolveHardenDeadlineMs()); - return result.success && existingAclIsCompliant(targetPath, directory, result.stdout, identity.name); - } catch { - return false; - } -} - -async function existingAclAlreadyCompliantAsync( - targetPath: string, - directory: boolean, -): Promise { - if (!shouldVerifyExistingAcl()) return false; - const identity = cachedCurrentWindowsIdentity(); - if (!identity) return false; - try { - const result = await asyncIcaclsRunner([targetPath], resolveHardenDeadlineMs()); - return result.success && existingAclIsCompliant(targetPath, directory, result.stdout, identity.name); - } catch { - return false; - } -} - function runIcacls(targetPath: string, directory: boolean, deadline: number): void { const principal = currentWindowsPrincipal(deadline); @@ -788,7 +724,6 @@ function hardenEntry( if (!existsSync(targetPath)) { cache.delete(targetPath); return { ok: true }; } if (effectivePlatform() !== "win32") return { ok: true }; if (memoSatisfied(cache, targetPath)) return { ok: true }; - if (existingAclAlreadyCompliant(targetPath, directory)) return { ok: true }; const memoKey = timeoutMemoKey(targetPath, opts); const timeoutMemoError = timeoutMemoErrorIfBlocked(memoKey, opts); if (timeoutMemoError) { @@ -841,7 +776,6 @@ async function hardenEntryAsync( if (!existsSync(targetPath)) { cache.delete(targetPath); return { ok: true }; } if (effectivePlatform() !== "win32") return { ok: true }; if (memoSatisfied(cache, targetPath)) return { ok: true }; - if (await existingAclAlreadyCompliantAsync(targetPath, directory)) return { ok: true }; const memoKey = timeoutMemoKey(targetPath, opts); const timeoutMemoError = timeoutMemoErrorIfBlocked(memoKey, opts); if (timeoutMemoError) { diff --git a/tests/windows-secret-acl.test.ts b/tests/windows-secret-acl.test.ts index e026d016f8..e72618198c 100644 --- a/tests/windows-secret-acl.test.ts +++ b/tests/windows-secret-acl.test.ts @@ -312,9 +312,8 @@ describe("effective Windows principal integration", () => { }); }); -describe("opt-in existing ACL proof", () => { +describe("existing ACL verification option", () => { const ownerSid = "S-1-5-21-111-222-333-1001"; - const ownerName = "EXAMPLE\\Owner"; const success = (stdout = ""): IcaclsResult => ({ success: true, exitCode: 0, @@ -322,19 +321,12 @@ describe("opt-in existing ACL proof", () => { stdout, }); - function seedIdentity(): void { - setWindowsPrincipalRunnerForTests(() => ({ - ...success(`${ownerSid}\r\n${ownerName}\r\n`), - })); - expect(resolveCurrentWindowsPrincipal(1_000)).toBe(`*${ownerSid}`); - } - beforeEach(() => { resetHardenedStateForTests(); resetWindowsPrincipalForTests(); setPlatformForTests("win32"); process.env.OPENCODEX_ACL_VERIFY_EXISTING = "1"; - seedIdentity(); + setWindowsPrincipalRunnerForTests(() => success(`${ownerSid}\nEXAMPLE\\Owner\n`)); }); afterEach(() => { @@ -347,106 +339,36 @@ describe("opt-in existing ACL proof", () => { resetWindowsPrincipalForTests(); }); - test("a same-line explicit owner ACE skips sync mutation", () => { - const target = join(testDir, "already-private.json"); + test("an owner-only ACL still disables inheritance synchronously", () => { + const target = join(testDir, "apparently-private.json"); writeFileSync(target, "secret"); const calls: string[][] = []; setIcaclsRunnerForTests(args => { calls.push(args); - return success(`${target} ${ownerName}:(F)\r\n\r\nSuccessfully processed 1 files; Failed processing 0 files\r\n`); + return success(`${target} EXAMPLE\\Owner:(F)\n`); }); expect(hardenSecretPath(target, { required: true })).toEqual({ ok: true }); - expect(calls).toEqual([[target]]); + expect(calls).toEqual([ + [target, "/grant:r", `*${ownerSid}:(F)`], + [target, "/inheritance:r"], + [target, "/remove:g", "*S-1-1-0", "*S-1-5-11", "*S-1-5-32-545"], + ]); }); - test("localized summary text and case-varied owner skip async directory mutation", async () => { - const target = join(testDir, "already-private-dir"); + test("an owner-only directory ACL still disables inheritance asynchronously", async () => { + const target = join(testDir, "apparently-private-dir"); mkdirSync(target); const calls: string[][] = []; - setAsyncWindowsPrincipalRunnerForTests(async () => success(`${ownerSid}\n${ownerName}\n`)); - seedIdentity(); + setAsyncWindowsPrincipalRunnerForTests(async () => success(`${ownerSid}\nEXAMPLE\\Owner\n`)); setAsyncIcaclsRunnerForTests(async args => { calls.push(args); - return success(`${target} ${ownerName.toLowerCase()}:(OI)(CI)(F)\r\n\r\n1 archivos procesados correctamente; error al procesar 0 archivos\r\n`); + return success(`${target} EXAMPLE\\Owner:(OI)(CI)(F)\n`); }); expect(await hardenSecretDirAsync(target, { required: true })).toEqual({ ok: true }); - expect(calls).toEqual([[target]]); - }); - - test("an inherited owner ACE falls through to the mutation sequence", () => { - const target = join(testDir, "inherited.json"); - writeFileSync(target, "secret"); - const calls: string[][] = []; - setIcaclsRunnerForTests(args => { - calls.push(args); - if (calls.length === 1) return success(`${target} ${ownerName}:(I)(F)\r\n`); - return success(); - }); - - expect(hardenSecretPath(target, { required: true })).toEqual({ ok: true }); - expect(calls.map(args => args[1] ?? "read")).toEqual(["read", "/grant:r", "/inheritance:r", "/remove:g"]); - }); - - test("an unexpected broad principal falls through to mutation", () => { - const target = join(testDir, "broad.json"); - writeFileSync(target, "secret"); - const calls: string[][] = []; - setIcaclsRunnerForTests(args => { - calls.push(args); - if (calls.length === 1) { - return success(`${target} ${ownerName}:(F)\r\n BUILTIN\\Users:(RX)\r\n`); - } - return success(); - }); - - expect(hardenSecretPath(target, { required: true })).toEqual({ ok: true }); - expect(calls.map(args => args[1] ?? "read")).toEqual(["read", "/grant:r", "/inheritance:r", "/remove:g"]); - }); - - test("an identity cache miss performs no read and uses the existing harden path", () => { - const target = join(testDir, "cold-identity.json"); - writeFileSync(target, "secret"); - resetWindowsPrincipalForTests(); - let identityCalls = 0; - const calls: string[][] = []; - setWindowsPrincipalRunnerForTests(() => { - identityCalls += 1; - return { ...success(`${ownerSid}\n${ownerName}\n`) }; - }); - setIcaclsRunnerForTests(args => { calls.push(args); return success(); }); - - expect(hardenSecretPath(target, { required: true })).toEqual({ ok: true }); - expect(identityCalls).toBe(1); - expect(calls.map(args => args[1])).toEqual(["/grant:r", "/inheritance:r", "/remove:g"]); - }); - - test("read-only success never enters the post-mutation memo", () => { - const target = join(testDir, "memo-free.json"); - writeFileSync(target, "secret"); - setIcaclsRunnerForTests(() => success(`${target} ${ownerName}:(F)\r\n`)); - - expect(hardenSecretPath(target, { required: true })).toEqual({ ok: true }); - expect(hardenedSecretPathCountForTests()).toBe(0); - delete process.env.OPENCODEX_ACL_VERIFY_EXISTING; - const mutations: string[][] = []; - setIcaclsRunnerForTests(args => { mutations.push(args); return success(); }); - expect(hardenSecretPath(target, { required: true })).toEqual({ ok: true }); - expect(mutations.map(args => args[1])).toEqual(["/grant:r", "/inheritance:r", "/remove:g"]); - expect(hardenedSecretPathCountForTests()).toBe(1); - }); - - test("the default flag-off path preserves the exact mutation sequence", () => { - const target = join(testDir, "default-mutation.json"); - writeFileSync(target, "secret"); - delete process.env.OPENCODEX_ACL_VERIFY_EXISTING; - const calls: string[][] = []; - setIcaclsRunnerForTests(args => { calls.push(args); return success(); }); - - expect(hardenSecretPath(target, { required: true })).toEqual({ ok: true }); expect(calls).toEqual([ - [target, "/grant:r", `*${ownerSid}:(F)`], + [target, "/grant:r", `*${ownerSid}:(OI)(CI)(F)`], [target, "/inheritance:r"], [target, "/remove:g", "*S-1-1-0", "*S-1-5-11", "*S-1-5-32-545"], ]);