fix: secure Windows reads reject foreign file owners

CHANGELOG.md

@@ -2,6 +2,23 @@
 
 ## 0.4.6 - Unreleased
 
+### Security and Correctness
+
+- Report Windows owner SIDs and whether the owner is the current user,
+  LocalSystem, or built-in Administrators so credential-bearing executable
+  checks can reject foreign-owned paths even when their visible DACL is
+  read-only. Secure reads enforce the result and remote filesystems fail
+  closed.
+- Read Windows owner and DACL data through the underlying .NET security
+  descriptor APIs so verification does not depend on PowerShell security-module
+  autoloading.
+- Invoke `icacls.exe` with its supported path-only inspection syntax and use
+  the live Windows user/domain environment for named ACE classification, so
+  ACL verification works on supported Windows hosts instead of failing on the
+  invalid `/sid` argument.
+- Normalize trailing Windows install-root separators with a bounded linear
+  scan so library-provided environment maps cannot trigger regex backtracking.
+
 ### Docs and Tooling
 
 - Add the public repository governance baseline, pinned CodeQL analysis,

docs/permissions.md

@@ -38,7 +38,7 @@ isWorldReadable(bits);
 isGroupReadable(bits);
 ```
 
-`inspectPathPermissions()` follows symlink targets for the effective mode but tells you whether the original path was a symlink. On POSIX it reports owner/group/world bits. On Windows it delegates to the ACL helpers below.
+`inspectPathPermissions()` follows symlink targets for the effective mode but tells you whether the original path was a symlink. On POSIX it reports owner/group/world bits. On Windows it delegates to the ACL helpers below and also reports `ownerSid` plus `ownerTrusted` when ownership can be verified. `ownerTrusted` is true only for a local volume owned by the current user, LocalSystem, or built-in Administrators; remote filesystems fail closed. Secure reads and callers that protect credential-bearing execution require `ownerTrusted === true`.
 
 ## Advanced Windows ACL helpers
 
@@ -82,6 +82,9 @@ type PermissionCheck = {
   groupWritable: boolean;
   worldReadable: boolean;
   groupReadable: boolean;
+  ownerSid?: string;
+  ownerTrusted?: boolean;
+  ownerError?: string;
   aclSummary?: string;
   error?: string;
 };

src/local-file-access.ts

@@ -21,7 +21,16 @@ export function isWindowsNetworkPath(
     return false;
   }
   const normalized = filePath.replace(/\//g, "\\");
-  return normalized.startsWith("\\\\?\\UNC\\") || normalized.startsWith("\\\\");
+  const extendedDrive =
+    normalized.length >= 7 &&
+    normalized.startsWith("\\\\?\\") &&
+    /^[a-z]$/i.test(normalized[4] ?? "") &&
+    normalized[5] === ":" &&
+    normalized[6] === "\\";
+  if (extendedDrive) {
+    return false;
+  }
+  return normalized.startsWith("\\\\");
 }
 
 export function isWindowsDriveLetterPath(

src/permissions.ts

@@ -5,6 +5,12 @@ import path from "node:path";
 import { promisify } from "node:util";
 
 import { normalizeLowercaseStringOrEmpty } from "./string-coerce.js";
+import { resolveWindowsSystemCommand } from "./windows-command.js";
+import {
+  inspectWindowsOwner,
+  resolveWindowsCurrentUserSid,
+  resolveWindowsPrincipalSids,
+} from "./windows-owner.js";
 
 const execFileAsync = promisify(execFile);
 
@@ -24,6 +30,12 @@ export type PermissionCheck = {
   groupWritable: boolean;
   worldReadable: boolean;
   groupReadable: boolean;
+  /** Canonical Windows owner SID when the owner query succeeds. */
+  ownerSid?: string;
+  /** Whether the Windows owner is the current user, LocalSystem, or Administrators. */
+  ownerTrusted?: boolean;
+  /** Owner-query failure detail when Windows ownership could not be verified. */
+  ownerError?: string;
   aclSummary?: string;
   error?: string;
 };
@@ -46,6 +58,8 @@ export type SafeStatResult = {
 
 export type WindowsAclEntry = {
   principal: string;
+  /** Canonical principal SID when resolved from Windows. */
+  sid?: string;
   rights: string[];
   rawRights: string;
   canRead: boolean;
@@ -172,7 +186,23 @@ export async function inspectPathPermissions(
   const bits = modeBits(effectiveMode);
   const platform = opts?.platform ?? process.platform;
   if (platform === "win32") {
-    const acl = await inspectWindowsAcl(targetPath, { env: opts?.env, exec: opts?.exec });
+    const owner = await inspectWindowsOwner({
+      targetPath,
+      env: opts?.env,
+      exec: opts?.exec ?? defaultPermissionExec,
+    });
+    const acl = await inspectWindowsAcl(targetPath, {
+      env: opts?.env,
+      exec: opts?.exec,
+      currentUserSid: owner.currentUserSid,
+      principalSids: owner.principalSids,
+      principalTranslationFailed: owner.principalTranslationFailed,
+    });
+    const ownerFields = {
+      ...(owner.sid ? { ownerSid: owner.sid } : {}),
+      ...(owner.trusted !== undefined ? { ownerTrusted: owner.trusted } : {}),
+      ...(owner.error ? { ownerError: owner.error } : {}),
+    };
     if (!acl.ok) {
       return {
         ok: true,
@@ -185,6 +215,7 @@ export async function inspectPathPermissions(
         groupWritable: false,
         worldReadable: false,
         groupReadable: false,
+        ...ownerFields,
         error: acl.error,
       };
     }
@@ -199,6 +230,7 @@ export async function inspectPathPermissions(
       groupWritable: acl.untrustedGroup.some((entry) => entry.canWrite),
       worldReadable: acl.untrustedWorld.some((entry) => entry.canRead),
       groupReadable: acl.untrustedGroup.some((entry) => entry.canRead),
+      ...ownerFields,
       aclSummary: formatWindowsAclSummary(acl),
     };
   }
@@ -298,42 +330,6 @@ function buildTrustedPrincipals(env?: NodeJS.ProcessEnv): Set<string> {
   return trusted;
 }
 
-function getEnvValueCaseInsensitive(env: NodeJS.ProcessEnv, name: string): string | undefined {
-  const direct = env[name];
-  if (direct !== undefined) {
-    return direct;
-  }
-  const lower = name.toLowerCase();
-  for (const [key, value] of Object.entries(env)) {
-    if (key.toLowerCase() === lower) {
-      return value;
-    }
-  }
-  return undefined;
-}
-
-function normalizeWindowsInstallRoot(value: string | undefined): string | null {
-  const trimmed = value?.trim();
-  if (!trimmed || !path.win32.isAbsolute(trimmed)) {
-    return null;
-  }
-  return trimmed.replace(/[\\/]+$/, "");
-}
-
-function resolveWindowsSystemRoot(env?: NodeJS.ProcessEnv): string {
-  const source = env ?? process.env;
-  return (
-    normalizeWindowsInstallRoot(getEnvValueCaseInsensitive(source, "SystemRoot")) ??
-    normalizeWindowsInstallRoot(getEnvValueCaseInsensitive(source, "WINDIR")) ??
-    "C:\\Windows"
-  );
-}
-
-function resolveWindowsSystemCommand(command: string, env?: NodeJS.ProcessEnv): string {
-  const root = resolveWindowsSystemRoot(env);
-  return path.win32.join(root, "System32", command);
-}
-
 function classifyPrincipal(
   principal: string,
   trustedPrincipals: Set<string>,
@@ -401,7 +397,14 @@ function parseAceEntry(entry: string): WindowsAclEntry | null {
   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;
-  return { principal, rights, rawRights, ...rightsFromTokens(rights) };
+  const normalizedPrincipal = normalizeSid(principal);
+  return {
+    principal,
+    ...(SID_RE.test(normalizedPrincipal) ? { sid: normalizedPrincipal } : {}),
+    rights,
+    rawRights,
+    ...rightsFromTokens(rights),
+  };
 }
 
 export function parseIcaclsOutput(output: string, targetPath: string): WindowsAclEntry[] {
@@ -440,52 +443,59 @@ export function summarizeWindowsAcl(
   const untrustedWorld: WindowsAclEntry[] = [];
   const untrustedGroup: WindowsAclEntry[] = [];
   for (const entry of entries) {
-    const classification = classifyPrincipal(entry.principal, trustedPrincipals);
+    const classification = classifyPrincipal(entry.sid ?? entry.principal, trustedPrincipals);
     if (classification === "trusted") trusted.push(entry);
     else if (classification === "world") untrustedWorld.push(entry);
     else untrustedGroup.push(entry);
   }
   return { trusted, untrustedWorld, untrustedGroup };
 }
 
-async function resolveCurrentUserSid(
-  exec: PermissionExec,
-  env?: NodeJS.ProcessEnv,
-): Promise<string | null> {
-  try {
-    const { stdout, stderr } = await exec(resolveWindowsSystemCommand("whoami.exe", env), [
-      "/user",
-      "/fo",
-      "csv",
-      "/nh",
-    ]);
-    const match = `${stdout}\n${stderr}`.match(/\*?S-\d+-\d+(?:-\d+)+/i);
-    return match ? normalizeSid(match[0]) : null;
-  } catch {
-    return null;
-  }
-}
-
 export async function inspectWindowsAcl(
   targetPath: string,
-  opts?: { env?: NodeJS.ProcessEnv; exec?: PermissionExec },
+  opts?: {
+    env?: NodeJS.ProcessEnv;
+    exec?: PermissionExec;
+    currentUserSid?: string;
+    principalSids?: Record<string, string>;
+    principalTranslationFailed?: boolean;
+  },
 ): Promise<WindowsAclSummary> {
   const exec = opts?.exec ?? defaultPermissionExec;
   try {
+    if (opts?.principalTranslationFailed) {
+      throw new Error("Windows ACL principal SID translation failed");
+    }
     const { stdout, stderr } = await exec(resolveWindowsSystemCommand("icacls.exe", opts?.env), [
       targetPath,
-      "/sid",
     ]);
-    const entries = parseIcaclsOutput(`${stdout}\n${stderr}`.trim(), targetPath);
-    let effectiveEnv = opts?.env;
+    let entries = parseIcaclsOutput(`${stdout}\n${stderr}`.trim(), targetPath);
+    const unresolvedPrincipals = entries
+      .filter((entry) => !entry.sid)
+      .map((entry) => entry.principal);
+    const principalSids = await resolveWindowsPrincipalSids({
+      principals: unresolvedPrincipals,
+      known: opts?.principalSids,
+      env: opts?.env,
+      exec,
+    });
+    entries = entries.map((entry) => {
+      const sid = entry.sid ?? principalSids[entry.principal.toLowerCase()];
+      if (!sid) {
+        throw new Error(`Windows ACL principal SID could not be verified: ${entry.principal}`);
+      }
+      return { ...entry, sid };
+    });
+    let currentUserSid = normalizeSid(opts?.currentUserSid ?? "");
+    let effectiveEnv = currentUserSid ? { USERSID: currentUserSid } : undefined;
     let { trusted, untrustedWorld, untrustedGroup } = summarizeWindowsAcl(entries, effectiveEnv);
     const needsUserSidResolution =
-      !effectiveEnv?.USERSID &&
-      untrustedGroup.some((entry) => SID_RE.test(normalize(entry.principal)));
+      !currentUserSid && untrustedGroup.some((entry) => entry.sid && !TRUSTED_SIDS.has(entry.sid));
     if (needsUserSidResolution) {
-      const currentUserSid = await resolveCurrentUserSid(exec, effectiveEnv);
+      currentUserSid =
+        (await resolveWindowsCurrentUserSid({ exec, env: opts?.env })) ?? "";
       if (currentUserSid) {
-        effectiveEnv = { ...effectiveEnv, USERSID: currentUserSid };
+        effectiveEnv = { USERSID: currentUserSid };
         ({ trusted, untrustedWorld, untrustedGroup } = summarizeWindowsAcl(entries, effectiveEnv));
       }
     }

src/secure-file.ts

@@ -186,6 +186,12 @@ async function assertSecurePermissions(
       `${label(options)} ACL verification unavailable on Windows for ${realPath}.`,
     );
   }
+  if (platform === "win32" && permissions.ownerTrusted !== true) {
+    throw new FsSafeError(
+      permissions.ownerTrusted === false ? "not-owned" : "permission-unverified",
+      `${label(options)} owner could not be trusted on Windows: ${realPath}`,
+    );
+  }
   const writableByOthers = permissions.worldWritable || permissions.groupWritable;
   const readableByOthers = permissions.worldReadable || permissions.groupReadable;
   if (writableByOthers || (!options.permissions?.allowReadableByOthers && readableByOthers)) {

src/windows-command.ts

@@ -0,0 +1,46 @@
+import path from "node:path";
+
+function getEnvValueCaseInsensitive(
+  env: NodeJS.ProcessEnv,
+  name: string,
+): string | undefined {
+  const direct = env[name];
+  if (direct !== undefined) {
+    return direct;
+  }
+  const lower = name.toLowerCase();
+  for (const [key, value] of Object.entries(env)) {
+    if (key.toLowerCase() === lower) {
+      return value;
+    }
+  }
+  return undefined;
+}
+
+function normalizeWindowsInstallRoot(value: string | undefined): string | null {
+  const trimmed = value?.trim();
+  if (!trimmed || !path.win32.isAbsolute(trimmed)) {
+    return null;
+  }
+  let end = trimmed.length;
+  while (end > 0 && (trimmed[end - 1] === "\\" || trimmed[end - 1] === "/")) {
+    end -= 1;
+  }
+  return trimmed.slice(0, end);
+}
+
+function resolveWindowsSystemRoot(env?: NodeJS.ProcessEnv): string {
+  const source = env ?? process.env;
+  return (
+    normalizeWindowsInstallRoot(getEnvValueCaseInsensitive(source, "SystemRoot")) ??
+    normalizeWindowsInstallRoot(getEnvValueCaseInsensitive(source, "WINDIR")) ??
+    "C:\\Windows"
+  );
+}
+
+export function resolveWindowsSystemCommand(
+  command: string,
+  env?: NodeJS.ProcessEnv,
+): string {
+  return path.win32.join(resolveWindowsSystemRoot(env), "System32", command);
+}