From 5fac371e35ba3032633daad90d22c6b3ce40261b Mon Sep 17 00:00:00 2001 From: Wenyu Date: Thu, 16 Jul 2026 06:06:49 -0400 Subject: [PATCH] fix: restrict ~/.openwiki ACLs on Windows where chmod is a no-op The OpenWiki home directory is created with mode 0o700 and chmod'd to 0o700 wherever it is ensured, but on Windows Node's chmod only toggles the read-only attribute - the directory holding ~/.openwiki/.env (every provider API key and OAuth refresh token in plaintext), the sqlite chat checkpoints, and raw connector dumps simply inherits whatever ACL its parent grants. restrictDirToCurrentUser mirrors the existing 0o700 owner-only intent with icacls: grant full control to the current user and SYSTEM (by well-known SID, language-independent), inheritable so new children are covered, then remove inherited ACEs. The grant runs before the inheritance reset so a failed grant can never lock the user out, and the whole helper is best-effort (returns false, never throws) exactly like the surrounding chmod calls. No-op on non-Windows platforms. Wired into ensureOpenWikiHome and saveOpenWikiEnv, the two places that express the 0o700 intent on the home directory. Verified on a real Windows 11 machine: a fresh temp directory carrying a dozen inherited modify-rights ACEs was reduced to exactly NT AUTHORITY\SYSTEM and the current user with (OI)(CI)(F). Co-Authored-By: Claude Fable 5 --- src/env.ts | 2 + src/openwiki-home.ts | 2 + src/windows-acl.ts | 41 +++++++++++++++++ test/windows-acl.test.ts | 97 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 142 insertions(+) create mode 100644 src/windows-acl.ts create mode 100644 test/windows-acl.test.ts diff --git a/src/env.ts b/src/env.ts index c38b422d..3bc5fa3d 100644 --- a/src/env.ts +++ b/src/env.ts @@ -52,6 +52,7 @@ import { resolveProviderRetryAttempts, } from "./constants.js"; import { isFileNotFoundError } from "./fs-errors.js"; +import { restrictDirToCurrentUser } from "./windows-acl.js"; export const openWikiEnvDir = path.join(os.homedir(), ".openwiki"); export const openWikiEnvPath = path.join(openWikiEnvDir, ".env"); @@ -208,6 +209,7 @@ export async function saveOpenWikiEnv(updates: EnvMap): Promise { mode: 0o700, }); await chmod(openWikiEnvDir, 0o700); + await restrictDirToCurrentUser(openWikiEnvDir); await writeFile(openWikiEnvPath, formatEnv(nextEnv), { encoding: "utf8", diff --git a/src/openwiki-home.ts b/src/openwiki-home.ts index 8c4420dc..bf641f77 100644 --- a/src/openwiki-home.ts +++ b/src/openwiki-home.ts @@ -1,6 +1,7 @@ import { chmod, mkdir } from "node:fs/promises"; import os from "node:os"; import path from "node:path"; +import { restrictDirToCurrentUser } from "./windows-acl.js"; export const openWikiHomeDir = path.join(os.homedir(), ".openwiki"); export const openWikiConnectorsDir = path.join(openWikiHomeDir, "connectors"); @@ -30,6 +31,7 @@ export function getConnectorLogsDir(connectorId: string): string { export async function ensureOpenWikiHome(): Promise { await mkdir(openWikiHomeDir, { recursive: true, mode: 0o700 }); await chmodIfExists(openWikiHomeDir, 0o700); + await restrictDirToCurrentUser(openWikiHomeDir); await mkdir(openWikiConnectorsDir, { recursive: true, mode: 0o700 }); await mkdir(openWikiLocalWikiDir, { recursive: true, mode: 0o700 }); await mkdir(openWikiSkillsDir, { recursive: true, mode: 0o700 }); diff --git a/src/windows-acl.ts b/src/windows-acl.ts new file mode 100644 index 00000000..7d7f5ca5 --- /dev/null +++ b/src/windows-acl.ts @@ -0,0 +1,41 @@ +import { execFile } from "node:child_process"; +import os from "node:os"; +import { promisify } from "node:util"; + +const execFileAsync = promisify(execFile); + +// Well-known SID for NT AUTHORITY\SYSTEM; the * prefix tells icacls it is a +// SID rather than an account name, so it resolves on any display language. +const SYSTEM_SID = "*S-1-5-18"; + +/** + * Mirrors the POSIX 0o700 owner-only intent on Windows, where fs.chmod only + * toggles the read-only attribute and leaves ACLs untouched: grants full + * control to the current user and SYSTEM (inheritable, so new children are + * covered), then removes inherited ACEs. The grant runs before the + * inheritance reset so a failed grant can never lock the user out of the + * directory. Best-effort by design: returns false instead of throwing so + * ACL tooling problems never block a run. No-op on non-Windows platforms. + */ +export async function restrictDirToCurrentUser( + dirPath: string, +): Promise { + if (process.platform !== "win32") { + return false; + } + + const userName = os.userInfo().username; + + try { + await execFileAsync("icacls", [ + dirPath, + "/grant:r", + `${userName}:(OI)(CI)F`, + `${SYSTEM_SID}:(OI)(CI)F`, + ]); + await execFileAsync("icacls", [dirPath, "/inheritance:r"]); + return true; + } catch { + return false; + } +} diff --git a/test/windows-acl.test.ts b/test/windows-acl.test.ts new file mode 100644 index 00000000..854df57d --- /dev/null +++ b/test/windows-acl.test.ts @@ -0,0 +1,97 @@ +import os from "node:os"; +import { afterEach, describe, expect, test, vi } from "vitest"; + +const execFileMock = vi.hoisted(() => + vi.fn( + ( + _command: string, + _args: string[], + callback: (error: Error | null, stdout: string, stderr: string) => void, + ) => { + callback(null, "", ""); + }, + ), +); + +vi.mock("node:child_process", () => ({ + execFile: execFileMock, +})); + +import { restrictDirToCurrentUser } from "../src/windows-acl.ts"; + +const realPlatform = process.platform; + +function setPlatform(platform: NodeJS.Platform): void { + Object.defineProperty(process, "platform", { + value: platform, + configurable: true, + }); +} + +afterEach(() => { + setPlatform(realPlatform); + execFileMock.mockClear(); +}); + +describe("restrictDirToCurrentUser", () => { + test("is a no-op on non-Windows platforms", async () => { + setPlatform("linux"); + + const restricted = await restrictDirToCurrentUser("/home/user/.openwiki"); + + expect(restricted).toBe(false); + expect(execFileMock).not.toHaveBeenCalled(); + }); + + test("grants the current user and SYSTEM, then removes inheritance", async () => { + setPlatform("win32"); + + const restricted = await restrictDirToCurrentUser( + "C:\\Users\\u\\.openwiki", + ); + + expect(restricted).toBe(true); + expect(execFileMock).toHaveBeenCalledTimes(2); + + const [grantCommand, grantArgs] = execFileMock.mock.calls[0] as unknown as [ + string, + string[], + ]; + expect(grantCommand).toBe("icacls"); + expect(grantArgs).toEqual([ + "C:\\Users\\u\\.openwiki", + "/grant:r", + `${os.userInfo().username}:(OI)(CI)F`, + "*S-1-5-18:(OI)(CI)F", + ]); + + const [, inheritanceArgs] = execFileMock.mock.calls[1] as unknown as [ + string, + string[], + ]; + expect(inheritanceArgs).toEqual([ + "C:\\Users\\u\\.openwiki", + "/inheritance:r", + ]); + }); + + test("does not remove inheritance when the grant fails, so a failed grant cannot lock the user out", async () => { + setPlatform("win32"); + execFileMock.mockImplementationOnce( + ( + _command: string, + _args: string[], + callback: (error: Error | null, stdout: string, stderr: string) => void, + ) => { + callback(new Error("icacls failed"), "", ""); + }, + ); + + const restricted = await restrictDirToCurrentUser( + "C:\\Users\\u\\.openwiki", + ); + + expect(restricted).toBe(false); + expect(execFileMock).toHaveBeenCalledTimes(1); + }); +});