Run the unit tests on Windows

foldersync/package.json

@@ -8,7 +8,7 @@
   "scripts": {
     "app": "npm run build && electron .",
     "build": "tsc -p tsconfig.json && node scripts/css-types.mjs && tsc -p tsconfig.renderer.json --noEmit && node scripts/build-renderer.mjs && node scripts/copy-static.mjs",
-    "build:tokenserver": "cd tokenserver && go build -o ../out/tokenserver .",
+    "build:tokenserver": "cd tokenserver && go build -o ../out/ .",
     "format": "prettier --write \"src/**/*.ts\" \"test/**/*.ts\" \"scripts/**/*.mjs\" \"*.mjs\" \"*.mts\"",
     "format:check": "prettier --check \"src/**/*.ts\" \"test/**/*.ts\" \"scripts/**/*.mjs\" \"*.mjs\" \"*.mts\"",
     "lint": "eslint . && stylelint \"src/**/*.css\"",

foldersync/scripts/start.mjs

@@ -18,7 +18,9 @@ if (!/^\d+$/.test(port) || Number(port) < 1 || Number(port) > 65535) {
 const tokenServerUrl = `http://127.0.0.1:${port}`;
 const home = process.env.FOLDERSYNC_HOME ?? join(homedir(), '.foldersync');
 const appKeyPath = join(home, 'tokenserver', 'app_key');
-const tokenServerBin = process.env.FOLDERSYNC_TOKENSERVER ?? join(root, 'out', 'tokenserver');
+const tokenServerBin =
+  process.env.FOLDERSYNC_TOKENSERVER ??
+  join(root, 'out', process.platform === 'win32' ? 'tokenserver.exe' : 'tokenserver');
 
 async function tokenServerRunning() {
   try {

foldersync/src/engine/contentHash.ts

@@ -1,8 +1,8 @@
 // Dropbox content_hash: SHA-256 of each 4 MiB block, then SHA-256 of the
 // concatenated block hashes, as hex.
 import {createHash, type Hash} from 'node:crypto';
-import {constants} from 'node:fs';
-import {open} from 'node:fs/promises';
+
+import {openNoFollow} from './openFile';
 
 export const BLOCK_SIZE = 4 * 1024 * 1024;
 
@@ -46,7 +46,7 @@ export function hashBuffer(data: Uint8Array): string {
 
 // Hashes a file. Fails if the path is a symlink, so it never reads outside the project.
 export async function hashFile(path: string): Promise<string> {
-  const file = await open(path, constants.O_RDONLY | constants.O_NOFOLLOW);
+  const file = await openNoFollow(path);
   try {
     const hasher = new ContentHasher();
     for await (const chunk of file.createReadStream({autoClose: false})) {

foldersync/src/engine/dropboxApi.ts

@@ -2,10 +2,10 @@
 // rpc() for other routes. Uploads send their data in parallel and commit in
 // batches, one batch at a time, since Dropbox limits how many writes run at
 // once in an account.
-import {constants} from 'node:fs';
 import {open, rm, stat} from 'node:fs/promises';
 
 import {ContentHasher, hashBuffer} from './contentHash';
+import {openNoFollow} from './openFile';
 
 export interface FileMeta {
   tag: 'file';
@@ -252,8 +252,8 @@ export class DropboxClient implements DropboxFiles {
     this.sendingData++;
     try {
       size = (await stat(req.localPath)).size;
-      // O_NOFOLLOW: if the file was swapped for a symlink, fail instead of uploading its target.
-      const file = await open(req.localPath, constants.O_RDONLY | constants.O_NOFOLLOW);
+      // If the file was swapped for a symlink, fail instead of uploading its target.
+      const file = await openNoFollow(req.localPath);
       try {
         sessionId = await this.uploadData(file, size, req);
       } finally {

foldersync/src/engine/openFile.ts

@@ -0,0 +1,31 @@
+// Opens a local file to read. Fails if the path itself is a symlink, or is
+// swapped for another file or a symlink while it's being opened. Callers check
+// that the path is inside the project.
+import {constants} from 'node:fs';
+import {lstat, open, type FileHandle} from 'node:fs/promises';
+
+export async function openNoFollow(path: string): Promise<FileHandle> {
+  const before = await lstat(path, {bigint: true});
+  if (before.isSymbolicLink()) {
+    throw new Error(`${path} is a symbolic link.`);
+  }
+  // Windows has no O_NOFOLLOW, so the checks below also cover a swap there.
+  const file = await open(path, constants.O_RDONLY | (constants.O_NOFOLLOW ?? 0));
+  try {
+    const opened = await file.stat({bigint: true});
+    const now = await lstat(path, {bigint: true});
+    if (
+      now.isSymbolicLink() ||
+      opened.ino !== before.ino ||
+      opened.dev !== before.dev ||
+      now.ino !== before.ino ||
+      now.dev !== before.dev
+    ) {
+      throw new Error(`${path} changed while it was being opened.`);
+    }
+    return file;
+  } catch (err) {
+    await file.close();
+    throw err;
+  }
+}

foldersync/test/account.test.ts

@@ -8,6 +8,7 @@ import {afterEach, beforeEach, describe, expect, it, vi} from 'vitest';
 import {Account, serverUrl} from '../src/main/account';
 import {Settings} from '../src/main/config';
 import type {TokenState} from '../src/shared/types';
+import {NO_CHMOD, chmodBlocksAccess} from './fsSupport';
 
 let dir: string;
 let credentialsPath: string;
@@ -153,7 +154,8 @@ describe('Account', () => {
     expect(redirects.every((r) => r === 'error')).toBe(true);
   });
 
-  it('keeps the previous account when settings can’t be saved', async () => {
+  it('keeps the previous account when settings can’t be saved', async ({skip}) => {
+    skip(!chmodBlocksAccess, NO_CHMOD);
     const server = twoUserServer();
     server.releaseA();
     const a = await account(server.fetchImpl);
@@ -171,7 +173,10 @@ describe('Account', () => {
     expect(() => a.userClient()).not.toThrow();
   });
 
-  it('stays signed out of a deleted account when its credentials can’t be removed', async () => {
+  it('stays signed out of a deleted account when its credentials can’t be removed', async ({
+    skip,
+  }) => {
+    skip(!chmodBlocksAccess, NO_CHMOD);
     const server = twoUserServer();
     server.releaseA();
     const a = await account(server.fetchImpl);
@@ -250,7 +255,8 @@ describe('Account', () => {
     expect(a.setupInfo().signedIn).toBe(false);
   });
 
-  it('stays signed in when the credentials file can’t be deleted', async () => {
+  it('stays signed in when the credentials file can’t be deleted', async ({skip}) => {
+    skip(!chmodBlocksAccess, NO_CHMOD);
     const server = tokenServer();
     server.release();
     const a = await account(server.fetchImpl);

foldersync/test/credentials.test.ts

@@ -5,6 +5,7 @@ import {join} from 'node:path';
 import {afterEach, beforeEach, describe, expect, it} from 'vitest';
 
 import {loadCredentials, saveCredentials, type Credentials} from '../src/engine/credentials';
+import {isWindows} from './fsSupport';
 
 let dir: string;
 let path: string;
@@ -35,7 +36,8 @@ describe('credentials file', () => {
     expect(await loadCredentials(path)).toEqual(creds);
   });
 
-  it('writes the file 0600 in a 0700 folder, even if they were looser', async () => {
+  it('writes the file 0600 in a 0700 folder, even if they were looser', async ({skip}) => {
+    skip(isWindows, 'Windows has no POSIX file modes');
     await chmod(join(dir, 'home'), 0o755);
     await writeFile(path, '{}', {mode: 0o644});
     await writeFile(`${path}.tmp`, 'old', {mode: 0o644});

foldersync/test/filesPanel.test.ts

@@ -1,6 +1,6 @@
 import {mkdir, mkdtemp, readFile, realpath, rm, symlink, writeFile} from 'node:fs/promises';
 import {tmpdir} from 'node:os';
-import {join} from 'node:path';
+import {basename, isAbsolute, join} from 'node:path';
 
 import {afterEach, beforeEach, describe, expect, it} from 'vitest';
 
@@ -167,7 +167,8 @@ describe('FilesPanel', () => {
       name: 'payload.exe',
     });
 
-    expect(opened.endsWith(`${'/'}a.txt`)).toBe(true);
+    expect(isAbsolute(opened)).toBe(true);
+    expect(basename(opened)).toBe('a.txt');
     panel.removeOpenCopies();
   });
 

foldersync/test/fsSupport.ts

@@ -0,0 +1,26 @@
+// What this computer's file system supports, so tests that need it can skip
+// where it's missing.
+import {chmodSync, mkdtempSync, readFileSync, rmSync, writeFileSync} from 'node:fs';
+import {tmpdir} from 'node:os';
+import {join} from 'node:path';
+
+// Whether chmod can stop this user reading a file. It can't on Windows, or as root.
+export const chmodBlocksAccess = ((): boolean => {
+  const dir = mkdtempSync(join(tmpdir(), 'foldersync-chmod-'));
+  const file = join(dir, 'probe');
+  writeFileSync(file, 'x');
+  chmodSync(file, 0o000);
+  try {
+    readFileSync(file);
+    return false;
+  } catch {
+    return true;
+  } finally {
+    chmodSync(file, 0o600);
+    rmSync(dir, {recursive: true, force: true});
+  }
+})();
+
+export const NO_CHMOD = 'chmod can’t block access on this computer';
+
+export const isWindows = process.platform === 'win32';

foldersync/test/openFile.test.ts

@@ -0,0 +1,89 @@
+import type {FileHandle} from 'node:fs/promises';
+import {lstat, mkdtemp, open, rename, rm, symlink, writeFile} from 'node:fs/promises';
+import {tmpdir} from 'node:os';
+import {join} from 'node:path';
+
+import {afterEach, beforeEach, describe, expect, it, vi} from 'vitest';
+
+import {openNoFollow} from '../src/engine/openFile';
+
+// The real functions, wrapped so a test can swap what lstat sees and look at
+// the handle open returned.
+vi.mock('node:fs/promises', async (importOriginal) => {
+  const real = await importOriginal<typeof import('node:fs/promises')>();
+  return {...real, lstat: vi.fn(real.lstat), open: vi.fn(real.open)};
+});
+
+let dir: string;
+
+beforeEach(async () => {
+  dir = await mkdtemp(join(tmpdir(), 'foldersync-open-'));
+  await writeFile(join(dir, 'a.txt'), 'A');
+  await writeFile(join(dir, 'b.txt'), 'B');
+});
+
+afterEach(async () => {
+  vi.mocked(lstat).mockClear();
+  vi.mocked(open).mockClear();
+  await rm(dir, {recursive: true, force: true});
+});
+
+describe('openNoFollow', () => {
+  it('opens a regular file', async () => {
+    const file = await openNoFollow(join(dir, 'a.txt'));
+    try {
+      expect((await file.readFile()).toString()).toBe('A');
+    } finally {
+      await file.close();
+    }
+  });
+
+  it('refuses a symlink without opening it', async () => {
+    await symlink(join(dir, 'a.txt'), join(dir, 'link.txt'));
+
+    await expect(openNoFollow(join(dir, 'link.txt'))).rejects.toThrow(/symbolic link/);
+    expect(open).not.toHaveBeenCalled();
+  });
+
+  it('refuses and closes a file that isn’t the one lstat saw', async () => {
+    // As if a.txt were swapped for b.txt between lstat and open.
+    vi.mocked(lstat).mockResolvedValueOnce(await lstat(join(dir, 'b.txt'), {bigint: true}));
+
+    await expect(openNoFollow(join(dir, 'a.txt'))).rejects.toThrow(
+      /changed while it was being opened/
+    );
+    const handle = (await vi.mocked(open).mock.results[0]?.value) as FileHandle;
+    expect(handle.fd).toBe(-1);
+  });
+
+  it('refuses and closes a file whose path is replaced right after it opens', async () => {
+    // lstat sees a.txt before the open, then b.txt, as if a.txt were replaced.
+    const a = await lstat(join(dir, 'a.txt'), {bigint: true});
+    const b = await lstat(join(dir, 'b.txt'), {bigint: true});
+    vi.mocked(lstat).mockResolvedValueOnce(a).mockResolvedValueOnce(b);
+
+    await expect(openNoFollow(join(dir, 'a.txt'))).rejects.toThrow(
+      /changed while it was being opened/
+    );
+    const handle = (await vi.mocked(open).mock.results[0]?.value) as FileHandle;
+    expect(handle.fd).toBe(-1);
+  });
+
+  it('refuses a file swapped for a symlink to itself while it opens', async () => {
+    const real = await vi.importActual<typeof import('node:fs/promises')>('node:fs/promises');
+    let handle: FileHandle | undefined;
+    // Moves a.txt and links its old name to it, right before the open.
+    vi.mocked(open).mockImplementationOnce(async (...args) => {
+      await rename(join(dir, 'a.txt'), join(dir, 'moved.txt'));
+      await symlink(join(dir, 'moved.txt'), join(dir, 'a.txt'));
+      handle = await real.open(...args);
+      return handle;
+    });
+
+    // macOS and Linux refuse the open itself (ELOOP); Windows opens it, then checks.
+    await expect(openNoFollow(join(dir, 'a.txt'))).rejects.toThrow(
+      /changed while it was being opened|ELOOP/
+    );
+    expect(handle?.fd ?? -1).toBe(-1);
+  });
+});

foldersync/test/projects.test.ts

@@ -10,6 +10,7 @@ import {ProjectClients, type UserApi} from '../src/main/projectClients';
 import {Projects} from '../src/main/projects';
 import type {SyncStatus} from '../src/shared/types';
 import type {DropboxFiles} from '../src/engine/dropboxApi';
+import {NO_CHMOD, chmodBlocksAccess} from './fsSupport';
 import {FakeRpc, agent, memoryDirectory} from './fakeRpc';
 
 const WAIT_MS = 10 * 60 * 1000;
@@ -428,7 +429,8 @@ describe('Projects.remove', () => {
     expect(existsSync(join(local, '.foldersync', 'state.db'))).toBe(true);
   });
 
-  it('says so when the project is removed but its sync state can’t be deleted', async () => {
+  it('says so when the project is removed but its sync state can’t be deleted', async ({skip}) => {
+    skip(!chmodBlocksAccess, NO_CHMOD);
     const {projects} = await setup(() => 'complete');
     await projects.setLocalDir('storyboard', join(dir, 'work', 'storyboard'));
     const state = join(dir, 'work', 'storyboard', '.foldersync');