diff --git a/packages/core/src/common/openai-client.ts b/packages/core/src/common/openai-client.ts index 4c903c7a..90c6256c 100644 --- a/packages/core/src/common/openai-client.ts +++ b/packages/core/src/common/openai-client.ts @@ -1,3 +1,4 @@ +import { createHash } from "crypto"; import * as fs from "fs"; import * as os from "os"; import * as path from "path"; @@ -72,7 +73,9 @@ export function createOpenAIClient(projectRoot: string = process.cwd()): { }; } - const cacheKey = `${connection.apiKey}::${connection.baseURL}`; + // Cache key hashes the apiKey so the raw secret never lingers in module + // state (a heap dump / crash report would otherwise contain it verbatim). + const cacheKey = `${createHash("sha256").update(connection.apiKey).digest("hex").slice(0, 16)}::${connection.baseURL}`; if (cachedOpenAI && cachedOpenAIKey === cacheKey) { return { client: cachedOpenAI, diff --git a/packages/core/src/common/private-storage.ts b/packages/core/src/common/private-storage.ts new file mode 100644 index 00000000..864ca24a --- /dev/null +++ b/packages/core/src/common/private-storage.ts @@ -0,0 +1,138 @@ +/** + * User-private filesystem helpers for DeepCode runtime state. + * + * DeepCode stores API keys and session data under the user's home directory. + * POSIX callers should rely on explicit mode bits (0600/0700) rather than the + * process umask, which is commonly permissive on desktop systems. Windows + * ignores POSIX mode bits, so we additionally restrict the NTFS ACL to the + * current user — matching the 0600 intent. + */ + +import childProcess from "child_process"; +import * as fs from "fs"; +import * as os from "os"; +import * as path from "path"; + +/** POSIX mode for private files: owner read/write only. */ +export const PRIVATE_FILE_MODE = 0o600; +/** POSIX mode for private directories: owner rwx only. */ +export const PRIVATE_DIRECTORY_MODE = 0o700; + +/** + * Resolve the current Windows user as a fully-qualified principal + * (``DOMAIN\user``) so ACL grants are unambiguous across machines/domains. + * + * ``USERDOMAIN``/``USERNAME`` win when present: they are set by Windows for + * every process, whereas under MSYS/Git-Bash a POSIX ``whoami`` earlier in + * PATH answers with a bare, ambiguous name. + * + * Returns null when the identity cannot be resolved (callers no-op). + */ +export function windowsIdentity(): string | null { + if (process.platform !== "win32") { + return null; + } + const { USERDOMAIN, USERNAME } = process.env; + if (USERDOMAIN && USERNAME) { + return `${USERDOMAIN}\\${USERNAME}`; + } + try { + const stdout = childProcess.execFileSync("whoami", { + encoding: "utf8", + timeout: 5000, + windowsHide: true, + stdio: ["ignore", "pipe", "ignore"], + }); + const principal = stdout.trim(); + return principal || null; + } catch { + return null; + } +} + +/** + * Restrict an NTFS path to the current user (Windows only; no-op elsewhere). + * + * Two idempotent steps, ordered fail-safe — grant before strip: + * 1. ``icacls /grant:r`` grants the current user exclusive full control + * (``:r`` replaces, does not append). For a directory the grant carries + * ``(OI)(CI)`` so it is inherited by existing and future children. + * 2. ``icacls /inheritance:r`` removes inherited ACEs so a permissive parent + * (e.g. the profile root granting ``Authenticated Users``) no longer + * applies. For a directory this propagates to children, which is why the + * grant above must already name this user. + * + * The order matters: if the grant is issued first and fails, the inherited + * ACEs are still in place and the path stays usable. Stripping first can + * leave the path (and, via inheritance, its children) with no ACE at all — the + * next open then throws ``EPERM`` and the state file becomes unreachable. + * + * Returns whether the ACL now grants the current user alone. Failures are + * reported, never thrown: callers decide whether an unprotected path is + * acceptable. Note the argv must not repeat the program name — passing + * ``icacls icacls `` makes icacls exit with ERROR_INVALID_PARAMETER (87) + * and leave the ACL untouched, which is how this helper silently no-opped + * before. + */ +export function restrictWindowsAcl(targetPath: string, isDirectory = false): boolean { + if (process.platform !== "win32") { + return true; + } + const identity = windowsIdentity(); + if (!identity) { + return false; + } + const grantee = isDirectory ? `${identity}:(OI)(CI)F` : `${identity}:F`; + const runIcacls = (args: string[]): boolean => { + try { + childProcess.execFileSync("icacls", args, { + encoding: "utf8", + timeout: 15000, + windowsHide: true, + stdio: ["ignore", "pipe", "ignore"], + }); + return true; + } catch { + return false; // best-effort: keep the caller moving, but do not pretend + } + }; + // Grant first: if this fails the inherited ACEs are still in place, so the + // path stays usable. Stripping first could leave a path with no ACE at all. + if (!runIcacls([targetPath, "/grant:r", grantee])) { + return false; + } + // Then drop the inherited ACEs. For a directory this propagates to + // children, which is why the grant above must already name this user. + if (!runIcacls([targetPath, "/inheritance:r"])) { + return false; + } + return true; +} + +/** + * Write a private file with user-only permissions on every platform. + * + * - POSIX: mode 0600 (applied by the write itself, subject to umask). + * - Windows: mode bits are ignored by the OS, so we grant the current user + * exclusive full control first and then remove inherited ACEs. + * + * Returns true when the platform's permission model was applied as requested. + */ +export function writePrivateFile(targetPath: string, contents: string): boolean { + fs.writeFileSync(targetPath, contents, { encoding: "utf8", mode: PRIVATE_FILE_MODE }); + return process.platform === "win32" ? restrictWindowsAcl(targetPath) : true; +} + +/** + * Ensure a directory exists with user-only permissions (0700 on POSIX; + * current-user-only ACL on Windows). Returns true when applied as requested. + */ +export function ensurePrivateDirectory(dirPath: string): boolean { + fs.mkdirSync(dirPath, { recursive: true, mode: PRIVATE_DIRECTORY_MODE }); + return process.platform === "win32" ? restrictWindowsAcl(dirPath, true) : true; +} + +/** Home directory used for DeepCode user state. */ +export function deepcodeHome(): string { + return path.join(os.homedir(), ".deepcode"); +} diff --git a/packages/core/src/settings.ts b/packages/core/src/settings.ts index 6bb59b97..1e4cabba 100644 --- a/packages/core/src/settings.ts +++ b/packages/core/src/settings.ts @@ -2,6 +2,7 @@ import { DEEPSEEK_V4_MODELS, defaultsToThinkingMode, type MultimodalMode } from import * as fs from "fs"; import * as os from "os"; import * as path from "path"; +import { ensurePrivateDirectory, writePrivateFile } from "./common/private-storage"; export type DeepcodingEnv = Record & { MODEL?: string; @@ -820,8 +821,8 @@ export function readProjectSettings(projectRoot: string = process.cwd()): Deepco } function writeSettingsFile(settingsPath: string, settings: DeepcodingSettings): void { - fs.mkdirSync(path.dirname(settingsPath), { recursive: true }); - fs.writeFileSync(settingsPath, `${JSON.stringify(settings, null, 2)}\n`, "utf8"); + ensurePrivateDirectory(path.dirname(settingsPath)); + writePrivateFile(settingsPath, `${JSON.stringify(settings, null, 2)}\n`); } export function writeSettings(settings: DeepcodingSettings): void { diff --git a/packages/core/src/tests/private-storage.test.ts b/packages/core/src/tests/private-storage.test.ts new file mode 100644 index 00000000..f6b2c30d --- /dev/null +++ b/packages/core/src/tests/private-storage.test.ts @@ -0,0 +1,201 @@ +import { afterEach, describe, it, type TestContext } from "node:test"; +import assert from "node:assert/strict"; +import childProcess from "child_process"; +import * as fs from "fs"; +import * as os from "os"; +import * as path from "path"; +import { + PRIVATE_FILE_MODE, + PRIVATE_DIRECTORY_MODE, + ensurePrivateDirectory, + restrictWindowsAcl, + windowsIdentity, + writePrivateFile, +} from "../common/private-storage"; + +const tempDirs: string[] = []; + +function tempDir(prefix: string): string { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), prefix)); + tempDirs.push(dir); + return dir; +} + +afterEach(() => { + for (const dir of tempDirs.splice(0)) { + fs.rmSync(dir, { recursive: true, force: true }); + } +}); + +/** + * Pretend to be Windows and record every execFileSync call. The platform + * property is patched so these regression tests run on every OS upstream CI + * uses — the icacls ordering bug is Windows-only and would otherwise get zero + * coverage off-Windows. ``restore()`` puts platform/env back; the mock itself + * is unwound by node:test at test end. + */ +function pretendWindows( + t: TestContext, + impl: (cmd: string, args: string[]) => string +): { calls: string[][]; restore: () => void } { + const platformDesc = Object.getOwnPropertyDescriptor(process, "platform"); + Object.defineProperty(process, "platform", { value: "win32", configurable: true }); + const prevDomain = process.env.USERDOMAIN; + const prevUser = process.env.USERNAME; + process.env.USERDOMAIN = "TESTDOM"; + process.env.USERNAME = "testuser"; + const calls: string[][] = []; + t.mock.method(childProcess, "execFileSync", (cmd: string, args: string[] = []) => { + calls.push([cmd, ...args]); + return impl(cmd, args); + }); + return { + calls, + restore: () => { + if (platformDesc) { + Object.defineProperty(process, "platform", platformDesc); + } + if (prevDomain === undefined) delete process.env.USERDOMAIN; + else process.env.USERDOMAIN = prevDomain; + if (prevUser === undefined) delete process.env.USERNAME; + else process.env.USERNAME = prevUser; + }, + }; +} + +describe("private-storage", () => { + it("writes files with 0600 mode on POSIX", () => { + if (process.platform === "win32") { + return; // mode bits are ignored on Windows + } + const dir = tempDir("dc-priv-posix-"); + const file = path.join(dir, "secret.json"); + // The strict return value is part of the contract: true means the caller + // has a private file, false must never masquerade as success. + assert.equal(writePrivateFile(file, "{}"), true, "writePrivateFile must report success"); + const mode = fs.statSync(file).mode & 0o777; + assert.equal(mode, PRIVATE_FILE_MODE, "file must be 0600"); + }); + + it("creates directories with 0700 mode on POSIX", () => { + if (process.platform === "win32") { + return; + } + const base = tempDir("dc-priv-dir-"); + const dir = path.join(base, ".deepcode", "nested"); + assert.equal(ensurePrivateDirectory(dir), true, "ensurePrivateDirectory must report success"); + const mode = fs.statSync(dir).mode & 0o777; + assert.equal(mode, PRIVATE_DIRECTORY_MODE, "directory must be 0700"); + }); + + it("restricts the Windows ACL to the current user (Windows only)", (t) => { + if (process.platform !== "win32") { + return; + } + const dir = tempDir("dc-priv-acl-"); + const file = path.join(dir, "credentials.json"); + const applied = writePrivateFile(file, "{}"); + if (!applied) { + // Say "not verified" out loud instead of passing vacuously: a file that + // icacls could not touch must never look like a green ACL test. + t.skip("icacls could not restrict the ACL in this environment"); + return; + } + + const out = childProcess.execFileSync("icacls", [file], { + encoding: "utf8", + windowsHide: true, + }); + + // 1. /inheritance:r really ran: not one inherited ACE may survive. This is + // the assertion that catches a silent no-op, where the file keeps the + // permissive ACEs it inherited from its parent directory. + assert.ok(!out.includes("(I)"), `inherited ACEs still apply:\n${out}`); + + // 2. The current user keeps full control. icacls prints `DOMAIN\user:(F)` + // for an explicit ACE and `DOMAIN\user:(I)(F)` when it is inherited, so + // read the exact principal back instead of matching a loose pattern. + const identity = windowsIdentity(); + assert.ok(identity, "windowsIdentity() must resolve the current user"); + assert.ok(out.includes(`${identity}:(F)`), `${identity} must keep (F):\n${out}`); + + // 3. No broad principal keeps access. These are the English Windows names; + // a localized build passes this trivially, which assertion 1 covers. + for (const broad of ["Authenticated Users", "Everyone"]) { + assert.ok(!out.includes(broad), `must not grant ${broad}:\n${out}`); + } + }); + + it("is idempotent when called repeatedly", () => { + const dir = tempDir("dc-priv-again-"); + const file = path.join(dir, "x.json"); + const first = writePrivateFile(file, "1"); + const second = writePrivateFile(file, "2"); + assert.equal(fs.readFileSync(file, "utf8"), "2", "last write must win"); + assert.equal(second, first, "repeated writes must report the same outcome"); + }); + + it("grants the user before stripping inheritance (fail-safe order)", (t) => { + // Regression: when the strip ran first, a failed grant left the path with + // no ACE at all and the next write threw EPERM (errno -4048). + const { calls, restore } = pretendWindows(t, () => ""); + try { + assert.equal(restrictWindowsAcl("C:\\state\\settings.json"), true, "clean run must report success"); + } finally { + restore(); + } + assert.deepEqual( + calls.map((args) => args.slice(1)), + [ + ["C:\\state\\settings.json", "/grant:r", "TESTDOM\\testuser:F"], + ["C:\\state\\settings.json", "/inheritance:r"], + ], + "the grant must be issued before the inheritance strip" + ); + }); + + it("keeps the inherited ACEs when the grant fails (no lockout)", (t) => { + const { calls, restore } = pretendWindows(t, (_cmd, args) => { + if (args.includes("/grant:r")) { + throw Object.assign(new Error("icacls: Access is denied."), { status: 5 }); + } + return ""; + }); + try { + assert.equal(restrictWindowsAcl("C:\\state\\settings.json"), false, "a failed grant must report failure"); + } finally { + restore(); + } + // Negative control: stripping after a failed grant is the lockout. The + // inherited ACEs must stay untouched so the path remains usable. + assert.ok( + calls.some((args) => args.includes("/grant:r")), + "the grant must have been attempted" + ); + assert.ok( + calls.every((args) => !args.includes("/inheritance:r")), + `inheritance must not be stripped after a failed grant: ${JSON.stringify(calls)}` + ); + }); + + it("directory grants are inheritable so children keep access", (t) => { + // Root cause companion: /inheritance:r on a directory propagates to its + // children, so the grant must already reach them ((OI)(CI)) or the state + // files inside become unopenable. + const { calls, restore } = pretendWindows(t, () => ""); + const dir = tempDir("dc-priv-dir-acl-"); + try { + assert.equal(ensurePrivateDirectory(path.join(dir, "deep")), true, "directory must report success"); + } finally { + restore(); + } + assert.deepEqual( + calls.map((args) => args.slice(1)), + [ + [path.join(dir, "deep"), "/grant:r", "TESTDOM\\testuser:(OI)(CI)F"], + [path.join(dir, "deep"), "/inheritance:r"], + ], + "directory grants must be inheritable and come first" + ); + }); +});