Merge 08a5c9378e into a3259400a3

browse/src/file-permissions.ts

@@ -42,6 +42,42 @@ import * as os from 'os';
 
 let warnedOnce = false;
 
+let cachedGrantee: string | undefined;
+
+/**
+ * Resolve the identity to use in icacls `/grant` arguments.
+ *
+ * Granting by bare username (`os.userInfo().username`) is ambiguous:
+ * LookupAccountName can resolve the name to the wrong principal when it
+ * collides with another account object — e.g. when the machine name equals
+ * the username (user "alice" on machine "ALICE"), the bare name can resolve
+ * to the machine's *domain* SID (S-1-5-21-… with no RID), producing an ACL
+ * that denies even the owner.
+ * A SID grant (`*S-1-5-21-…-1001`) is unambiguous, so we resolve the
+ * current user's SID via `whoami /user` and fall back to the fully
+ * qualified `DOMAIN\username` (still unambiguous) only if that fails.
+ */
+function getWindowsGrantee(): string {
+  if (cachedGrantee !== undefined) return cachedGrantee;
+  try {
+    // Absolute path: a bare `whoami` can resolve to GNU coreutils' whoami
+    // (Git for Windows / MSYS2 put usr/bin on PATH), which rejects /user.
+    const whoami = `${process.env.SystemRoot || 'C:\\Windows'}\\System32\\whoami.exe`;
+    const out = execFileSync(whoami, ['/user', '/fo', 'csv', '/nh'], {
+      encoding: 'utf8',
+      stdio: ['ignore', 'pipe', 'ignore'],
+    });
+    const sid = out.match(/"(S-1-[0-9-]+)"/)?.[1];
+    if (sid) {
+      cachedGrantee = `*${sid}`;
+      return cachedGrantee;
+    }
+  } catch { /* fall through to name-based grant */ }
+  const user = process.env.USERNAME || os.userInfo().username;
+  cachedGrantee = process.env.USERDOMAIN ? `${process.env.USERDOMAIN}\\${user}` : user;
+  return cachedGrantee;
+}
+
 function warnIcaclsFailure(fsPath: string, err: unknown): void {
   if (warnedOnce) return;
   warnedOnce = true;
@@ -67,7 +103,7 @@ function warnIcaclsFailure(fsPath: string, err: unknown): void {
 export function restrictFilePermissions(filePath: string): void {
   if (process.platform === 'win32') {
     try {
-      const user = os.userInfo().username;
+      const user = getWindowsGrantee();
       execFileSync(
         'icacls',
         [filePath, '/inheritance:r', '/grant:r', `${user}:(F)`],
@@ -97,7 +133,7 @@ export function restrictFilePermissions(filePath: string): void {
 export function restrictDirectoryPermissions(dirPath: string): void {
   if (process.platform === 'win32') {
     try {
-      const user = os.userInfo().username;
+      const user = getWindowsGrantee();
       execFileSync(
         'icacls',
         [dirPath, '/inheritance:r', '/grant:r', `${user}:(OI)(CI)(F)`],
@@ -155,3 +191,10 @@ export function mkdirSecure(dirPath: string): void {
 export function __resetWarnedForTests(): void {
   warnedOnce = false;
 }
+
+/**
+ * Expose the resolved icacls grantee. Test-only.
+ */
+export function __getWindowsGranteeForTests(): string {
+  return getWindowsGrantee();
+}

browse/test/file-permissions.test.ts

@@ -23,6 +23,7 @@ import {
   appendSecureFile,
   mkdirSecure,
   __resetWarnedForTests,
+  __getWindowsGranteeForTests,
 } from '../src/file-permissions';
 
 let tmpDir: string;
@@ -36,6 +37,20 @@ afterEach(() => {
   try { fs.rmSync(tmpDir, { recursive: true, force: true }); } catch { /* best-effort */ }
 });
 
+describe('getWindowsGrantee', () => {
+  test('on Windows, resolves an unambiguous grantee (SID or DOMAIN\\user)', () => {
+    if (process.platform !== 'win32') return;
+    // A bare-username grant is ambiguous when the username collides with
+    // another account object (e.g. machine name == username), which locks
+    // even the owner out. The grantee must be a SID (`*S-1-...`) or, as a
+    // fallback, a domain-qualified name — never a bare username.
+    const grantee = __getWindowsGranteeForTests();
+    const isSid = /^\*S-1-[0-9-]+$/.test(grantee);
+    const isQualified = grantee.includes('\\');
+    expect(isSid || isQualified).toBe(true);
+  });
+});
+
 describe('restrictFilePermissions', () => {
   test('on POSIX, sets file mode to 0o600', () => {
     if (process.platform === 'win32') return;