diff --git a/src/security/windows-acl.test.ts b/src/security/windows-acl.test.ts index 69d75e5c6..5318e3096 100644 --- a/src/security/windows-acl.test.ts +++ b/src/security/windows-acl.test.ts @@ -18,6 +18,22 @@ const { summarizeWindowsAcl, } = await import("./windows-acl.js"); +function aclEntry(params: { + principal: string; + rights?: string[]; + rawRights?: string; + canRead?: boolean; + canWrite?: boolean; +}): WindowsAclEntry { + return { + principal: params.principal, + rights: params.rights ?? ["F"], + rawRights: params.rawRights ?? "(F)", + canRead: params.canRead ?? true, + canWrite: params.canWrite ?? true, + }; +} + describe("windows-acl", () => { describe("resolveWindowsUserPrincipal", () => { it("returns DOMAIN\\USERNAME when both are present", () => { @@ -81,6 +97,7 @@ Successfully processed 1 files`; it("skips status messages", () => { const output = `Successfully processed 1 files + Processed file: C:\\test\\file.txt Failed processing 0 files No mapping between account names`; const entries = parseIcaclsOutput(output, "C:\\test\\file.txt"); @@ -107,6 +124,14 @@ Successfully processed 1 files`; expect(entries[1].principal).toBe("S-1-5-21-1824257776-4070701511-781240313-1001"); }); + it("ignores malformed ACL lines that contain ':' but no rights tokens", () => { + const output = `C:\\test\\file.txt random:message + C:\\test\\file.txt BUILTIN\\Administrators:(F)`; + const entries = parseIcaclsOutput(output, "C:\\test\\file.txt"); + expect(entries).toHaveLength(1); + expect(entries[0].principal).toBe("BUILTIN\\Administrators"); + }); + it("handles quoted target paths", () => { const output = `"C:\\path with spaces\\file.txt" BUILTIN\\Administrators:(F)`; const entries = parseIcaclsOutput(output, "C:\\path with spaces\\file.txt"); @@ -140,20 +165,8 @@ Successfully processed 1 files`; describe("summarizeWindowsAcl", () => { it("classifies trusted principals", () => { const entries: WindowsAclEntry[] = [ - { - principal: "NT AUTHORITY\\SYSTEM", - rights: ["F"], - rawRights: "(F)", - canRead: true, - canWrite: true, - }, - { - principal: "BUILTIN\\Administrators", - rights: ["F"], - rawRights: "(F)", - canRead: true, - canWrite: true, - }, + aclEntry({ principal: "NT AUTHORITY\\SYSTEM" }), + aclEntry({ principal: "BUILTIN\\Administrators" }), ]; const summary = summarizeWindowsAcl(entries); expect(summary.trusted).toHaveLength(2); @@ -163,20 +176,8 @@ Successfully processed 1 files`; it("classifies world principals", () => { const entries: WindowsAclEntry[] = [ - { - principal: "Everyone", - rights: ["R"], - rawRights: "(R)", - canRead: true, - canWrite: false, - }, - { - principal: "BUILTIN\\Users", - rights: ["R"], - rawRights: "(R)", - canRead: true, - canWrite: false, - }, + aclEntry({ principal: "Everyone", rights: ["R"], rawRights: "(R)", canWrite: false }), + aclEntry({ principal: "BUILTIN\\Users", rights: ["R"], rawRights: "(R)", canWrite: false }), ]; const summary = summarizeWindowsAcl(entries); expect(summary.trusted).toHaveLength(0); @@ -185,15 +186,7 @@ Successfully processed 1 files`; }); it("classifies current user as trusted", () => { - const entries: WindowsAclEntry[] = [ - { - principal: "WORKGROUP\\TestUser", - rights: ["F"], - rawRights: "(F)", - canRead: true, - canWrite: true, - }, - ]; + const entries: WindowsAclEntry[] = [aclEntry({ principal: "WORKGROUP\\TestUser" })]; const env = { USERNAME: "TestUser", USERDOMAIN: "WORKGROUP" }; const summary = summarizeWindowsAcl(entries, env); expect(summary.trusted).toHaveLength(1); @@ -217,15 +210,7 @@ Successfully processed 1 files`; describe("summarizeWindowsAcl — SID-based classification", () => { it("classifies SYSTEM SID (S-1-5-18) as trusted", () => { - const entries: WindowsAclEntry[] = [ - { - principal: "S-1-5-18", - rights: ["F"], - rawRights: "(F)", - canRead: true, - canWrite: true, - }, - ]; + const entries: WindowsAclEntry[] = [aclEntry({ principal: "S-1-5-18" })]; const summary = summarizeWindowsAcl(entries); expect(summary.trusted).toHaveLength(1); expect(summary.untrustedWorld).toHaveLength(0); @@ -233,15 +218,7 @@ Successfully processed 1 files`; }); it("classifies BUILTIN\\Administrators SID (S-1-5-32-544) as trusted", () => { - const entries: WindowsAclEntry[] = [ - { - principal: "S-1-5-32-544", - rights: ["F"], - rawRights: "(F)", - canRead: true, - canWrite: true, - }, - ]; + const entries: WindowsAclEntry[] = [aclEntry({ principal: "S-1-5-32-544" })]; const summary = summarizeWindowsAcl(entries); expect(summary.trusted).toHaveLength(1); expect(summary.untrustedGroup).toHaveLength(0); @@ -249,21 +226,23 @@ Successfully processed 1 files`; it("classifies caller SID from USERSID env var as trusted", () => { const callerSid = "S-1-5-21-1824257776-4070701511-781240313-1001"; - const entries: WindowsAclEntry[] = [ - { - principal: callerSid, - rights: ["F"], - rawRights: "(F)", - canRead: true, - canWrite: true, - }, - ]; + const entries: WindowsAclEntry[] = [aclEntry({ principal: callerSid })]; const env = { USERSID: callerSid }; const summary = summarizeWindowsAcl(entries, env); expect(summary.trusted).toHaveLength(1); expect(summary.untrustedGroup).toHaveLength(0); }); + it("matches SIDs case-insensitively and trims USERSID", () => { + const entries: WindowsAclEntry[] = [ + aclEntry({ principal: "s-1-5-21-1824257776-4070701511-781240313-1001" }), + ]; + const env = { USERSID: " S-1-5-21-1824257776-4070701511-781240313-1001 " }; + const summary = summarizeWindowsAcl(entries, env); + expect(summary.trusted).toHaveLength(1); + expect(summary.untrustedGroup).toHaveLength(0); + }); + it("classifies unknown SID as group (not world)", () => { const entries: WindowsAclEntry[] = [ { diff --git a/src/security/windows-acl.ts b/src/security/windows-acl.ts index 8852ee2b7..f376db284 100644 --- a/src/security/windows-acl.ts +++ b/src/security/windows-acl.ts @@ -43,6 +43,12 @@ const TRUSTED_SIDS = new Set([ "s-1-5-32-544", "s-1-5-80-956008885-3418522649-1831038044-1853292631-2271478464", ]); +const STATUS_PREFIXES = [ + "successfully processed", + "processed", + "failed processing", + "no mapping between account names", +]; const normalize = (value: string) => value.trim().toLowerCase(); @@ -66,7 +72,7 @@ function buildTrustedPrincipals(env?: NodeJS.ProcessEnv): Set { trusted.add(normalize(userOnly)); } } - const userSid = env?.USERSID?.trim().toLowerCase(); + const userSid = normalize(env?.USERSID ?? ""); if (userSid && SID_RE.test(userSid)) { trusted.add(userSid); } @@ -75,19 +81,24 @@ function buildTrustedPrincipals(env?: NodeJS.ProcessEnv): Set { function classifyPrincipal( principal: string, - env?: NodeJS.ProcessEnv, + trustedPrincipals: Set, ): "trusted" | "world" | "group" { const normalized = normalize(principal); - const trusted = buildTrustedPrincipals(env); if (SID_RE.test(normalized)) { - return TRUSTED_SIDS.has(normalized) || trusted.has(normalized) ? "trusted" : "group"; + return TRUSTED_SIDS.has(normalized) || trustedPrincipals.has(normalized) ? "trusted" : "group"; } - if (trusted.has(normalized) || TRUSTED_SUFFIXES.some((s) => normalized.endsWith(s))) { + if ( + trustedPrincipals.has(normalized) || + TRUSTED_SUFFIXES.some((suffix) => normalized.endsWith(suffix)) + ) { return "trusted"; } - if (WORLD_PRINCIPALS.has(normalized) || WORLD_SUFFIXES.some((s) => normalized.endsWith(s))) { + if ( + WORLD_PRINCIPALS.has(normalized) || + WORLD_SUFFIXES.some((suffix) => normalized.endsWith(suffix)) + ) { return "world"; } return "group"; @@ -101,6 +112,58 @@ function rightsFromTokens(tokens: string[]): { canRead: boolean; canWrite: boole return { canRead, canWrite }; } +function isStatusLine(lowerLine: string): boolean { + return STATUS_PREFIXES.some((prefix) => lowerLine.startsWith(prefix)); +} + +function stripTargetPrefix(params: { + trimmedLine: string; + lowerLine: string; + normalizedTarget: string; + lowerTarget: string; + quotedTarget: string; + quotedLower: string; +}): string { + if (params.lowerLine.startsWith(params.lowerTarget)) { + return params.trimmedLine.slice(params.normalizedTarget.length).trim(); + } + if (params.lowerLine.startsWith(params.quotedLower)) { + return params.trimmedLine.slice(params.quotedTarget.length).trim(); + } + return params.trimmedLine; +} + +function parseAceEntry(entry: string): WindowsAclEntry | null { + if (!entry || !entry.includes("(")) { + return null; + } + + const idx = entry.indexOf(":"); + if (idx === -1) { + return null; + } + + const principal = entry.slice(0, idx).trim(); + const rawRights = entry.slice(idx + 1).trim(); + const tokens = + rawRights + .match(/\(([^)]+)\)/g) + ?.map((token) => token.slice(1, -1).trim()) + .filter(Boolean) ?? []; + + if (tokens.some((token) => token.toUpperCase() === "DENY")) { + return null; + } + + const rights = tokens.filter((token) => !INHERIT_FLAGS.has(token.toUpperCase())); + if (rights.length === 0) { + return null; + } + + const { canRead, canWrite } = rightsFromTokens(rights); + return { principal, rights, rawRights, canRead, canWrite }; +} + export function parseIcaclsOutput(output: string, targetPath: string): WindowsAclEntry[] { const entries: WindowsAclEntry[] = []; const normalizedTarget = targetPath.trim(); @@ -115,50 +178,23 @@ export function parseIcaclsOutput(output: string, targetPath: string): WindowsAc } const trimmed = line.trim(); const lower = trimmed.toLowerCase(); - if ( - lower.startsWith("successfully processed") || - lower.startsWith("processed") || - lower.startsWith("failed processing") || - lower.startsWith("no mapping between account names") - ) { + if (isStatusLine(lower)) { continue; } - let entry = trimmed; - if (lower.startsWith(lowerTarget)) { - entry = trimmed.slice(normalizedTarget.length).trim(); - } else if (lower.startsWith(quotedLower)) { - entry = trimmed.slice(quotedTarget.length).trim(); - } - if (!entry) { + const entry = stripTargetPrefix({ + trimmedLine: trimmed, + lowerLine: lower, + normalizedTarget, + lowerTarget, + quotedTarget, + quotedLower, + }); + const parsed = parseAceEntry(entry); + if (!parsed) { continue; } - - if (!entry.includes("(")) { - continue; - } - - const idx = entry.indexOf(":"); - if (idx === -1) { - continue; - } - - const principal = entry.slice(0, idx).trim(); - const rawRights = entry.slice(idx + 1).trim(); - const tokens = - rawRights - .match(/\(([^)]+)\)/g) - ?.map((token) => token.slice(1, -1).trim()) - .filter(Boolean) ?? []; - if (tokens.some((token) => token.toUpperCase() === "DENY")) { - continue; - } - const rights = tokens.filter((token) => !INHERIT_FLAGS.has(token.toUpperCase())); - if (rights.length === 0) { - continue; - } - const { canRead, canWrite } = rightsFromTokens(rights); - entries.push({ principal, rights, rawRights, canRead, canWrite }); + entries.push(parsed); } return entries; @@ -168,11 +204,12 @@ export function summarizeWindowsAcl( entries: WindowsAclEntry[], env?: NodeJS.ProcessEnv, ): Pick { + const trustedPrincipals = buildTrustedPrincipals(env); const trusted: WindowsAclEntry[] = []; const untrustedWorld: WindowsAclEntry[] = []; const untrustedGroup: WindowsAclEntry[] = []; for (const entry of entries) { - const classification = classifyPrincipal(entry.principal, env); + const classification = classifyPrincipal(entry.principal, trustedPrincipals); if (classification === "trusted") { trusted.push(entry); } else if (classification === "world") {