diff --git a/src/cli/embeddings.ts b/src/cli/embeddings.ts index 6dcfc16b..5d28203a 100644 --- a/src/cli/embeddings.ts +++ b/src/cli/embeddings.ts @@ -4,7 +4,7 @@ import { userInfo } from "node:os"; import { join } from "node:path"; import { HOME, ensureDir, log, pkgRoot, symlinkForce, warn, writeJson } from "./util.js"; import { pidPathFor, socketPathFor } from "../embeddings/protocol.js"; -import { getEmbeddingsEnabled, setEmbeddingsEnabled } from "../user-config.js"; +import { assertUserConfigWritable, getEmbeddingsEnabled, setEmbeddingsEnabled } from "../user-config.js"; import { ensureGraphDeps } from "./graph-deps.js"; /** @@ -176,6 +176,7 @@ function linkAgent(install: AgentInstall): void { * prior `disable`). */ export function installEmbeddings(): void { + assertUserConfigWritable(); ensureSharedDeps(); // Provision the code-graph parsers into the same shared dir so the // graph-on-stop hook (which symlinks here) can auto-build the graph. @@ -216,6 +217,7 @@ export function enableEmbeddings(): void { * effect immediately. Counterpart to `install`. */ export function uninstallEmbeddings(opts?: { prune?: boolean }): void { + assertUserConfigWritable(); const installs = findHivemindInstalls(); for (const inst of installs) { const link = join(inst.pluginDir, "node_modules"); diff --git a/src/cli/install-codex.ts b/src/cli/install-codex.ts index 64a01ad3..dd6df248 100644 --- a/src/cli/install-codex.ts +++ b/src/cli/install-codex.ts @@ -170,19 +170,33 @@ export function mergeHooks( } // Filesystem-bound wrapper: reads HOOKS_PATH (if present) and feeds the -// parsed result to the pure mergeHooks. Catches malformed JSON and warns. +// parsed result to the pure mergeHooks. Malformed JSON is an install blocker: +// replacing it with an empty object would destroy user hooks. // Also surfaces a warning listing any foreign-path hivemind entries // stripped (e.g. a dev clone wired in under a different directory). -function mergeHooksJson(ours: Record): Record { - let existing: Record = {}; +function readHooksJson(): Record { + let raw: string; try { - if (existsSync(HOOKS_PATH)) { - const parsed = JSON.parse(readFileSync(HOOKS_PATH, "utf-8")); - if (parsed && typeof parsed === "object") existing = parsed as Record; - } + raw = readFileSync(HOOKS_PATH, "utf-8"); + } catch (err) { + if (typeof err === "object" && err !== null && "code" in err && err.code === "ENOENT") return {}; + const detail = err instanceof Error && err.message ? `: ${err.message}` : ""; + throw new Error(`Codex hooks config at ${HOOKS_PATH} could not be read${detail}`); + } + let parsed: unknown; + try { + parsed = JSON.parse(raw); } catch { - warn(` Codex ${HOOKS_PATH} unparseable — ignoring prior content`); + throw new Error(`Codex hooks config at ${HOOKS_PATH} is not valid JSON; fix or remove it, then rerun.`); + } + if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) { + throw new Error(`Codex hooks config at ${HOOKS_PATH} must contain a JSON object; fix or remove it, then rerun.`); } + return parsed as Record; +} + +function mergeHooksJson(ours: Record): Record { + const existing = readHooksJson(); reportForeignHivemindHooks(existing); return mergeHooks(existing, ours); } @@ -299,6 +313,9 @@ export function installCodex(): void { throw new Error(`Codex bundle missing at ${srcBundle}. Run 'npm run build' first.`); } + // Validate before copying payloads or changing Codex feature flags. + const mergedHooks = mergeHooksJson(buildHooksJson()); + ensureDir(PLUGIN_DIR); reportPruned("Codex", syncDir(srcBundle, join(PLUGIN_DIR, "bundle"))); if (existsSync(srcSkills)) reportPruned("Codex", syncDir(srcSkills, join(PLUGIN_DIR, "skills"))); @@ -309,7 +326,7 @@ export function installCodex(): void { // Codex fingerprints and re-triggers its "Hooks need review" trust prompt on // every install/update. Skipping the no-op write keeps the user from being // re-prompted each time. - if (!writeJsonIfChanged(HOOKS_PATH, mergeHooksJson(buildHooksJson()))) { + if (!writeJsonIfChanged(HOOKS_PATH, mergedHooks)) { log(` Codex hooks.json unchanged — skipped rewrite (no re-trust prompt)`); } @@ -342,34 +359,22 @@ export function installCodex(): void { } export function uninstallCodex(): void { - if (existsSync(HOOKS_PATH)) { - // Symmetric with install: strip ONLY our hivemind entries via mergeHooks. - // The pre-fix unconditional unlinkSync(HOOKS_PATH) destroyed any user- - // defined hooks (e.g. a custom Notification handler) that lived alongside - // ours. mergeHooks(existing, { hooks: {} }) preserves the user's events - // and removes only the ones whose command points into PLUGIN_DIR/bundle/. - let existing: Record = {}; - try { - const raw = JSON.parse(readFileSync(HOOKS_PATH, "utf-8")); - if (raw && typeof raw === "object") existing = raw as Record; - } catch { - // Malformed JSON: fall back to deleting the file rather than guess at - // intent. Same behavior as pre-fix; user can recreate cleanly. + // Symmetric with install: strip ONLY our hivemind entries via mergeHooks. + // The pre-fix unconditional unlinkSync(HOOKS_PATH) destroyed any user- + // defined hooks (e.g. a custom Notification handler) that lived alongside + // ours. Validate first so malformed/unreadable config leaves every other + // uninstall target untouched and propagates a failure to the dispatcher. + const existing = readHooksJson(); + if (Object.keys(existing).length > 0) { + const stripped = mergeHooks(existing, { hooks: {} }); + const survivingHooks = (stripped.hooks ?? {}) as Record; + const otherTopLevelKeys = Object.keys(stripped).filter(k => k !== "hooks"); + if (Object.keys(survivingHooks).length === 0 && otherTopLevelKeys.length === 0) { unlinkSync(HOOKS_PATH); - log(` Codex removed unparseable ${HOOKS_PATH}`); - existing = {}; - } - if (Object.keys(existing).length > 0) { - const stripped = mergeHooks(existing, { hooks: {} }); - const survivingHooks = (stripped.hooks ?? {}) as Record; - const otherTopLevelKeys = Object.keys(stripped).filter(k => k !== "hooks"); - if (Object.keys(survivingHooks).length === 0 && otherTopLevelKeys.length === 0) { - unlinkSync(HOOKS_PATH); - log(` Codex removed ${HOOKS_PATH}`); - } else { - writeJson(HOOKS_PATH, stripped); - log(` Codex stripped hivemind hooks from ${HOOKS_PATH}`); - } + log(` Codex removed ${HOOKS_PATH}`); + } else { + writeJson(HOOKS_PATH, stripped); + log(` Codex stripped hivemind hooks from ${HOOKS_PATH}`); } } if (existsSync(SKILL_LINK)) { diff --git a/src/cli/install-cursor.ts b/src/cli/install-cursor.ts index 067198e5..5eade345 100644 --- a/src/cli/install-cursor.ts +++ b/src/cli/install-cursor.ts @@ -1,6 +1,6 @@ -import { existsSync, lstatSync, rmSync, unlinkSync } from "node:fs"; +import { existsSync, lstatSync, rmSync, unlinkSync, readFileSync } from "node:fs"; import { join } from "node:path"; -import { HOME, pkgRoot, ensureDir, syncDir, readJson, writeJson, writeJsonIfChanged, symlinkForce, writeVersionStamp, log, reportPruned } from "./util.js"; +import { HOME, pkgRoot, ensureDir, syncDir, writeJson, writeJsonIfChanged, symlinkForce, writeVersionStamp, log, reportPruned } from "./util.js"; import { getVersion } from "./version.js"; // Cursor 1.7+ hooks API: https://cursor.com/docs/agent/hooks @@ -84,6 +84,27 @@ function mergeHooks(existing: Record | null): Record; } +function readHooksConfig(): Record | null { + let raw: string; + try { + raw = readFileSync(HOOKS_PATH, "utf-8"); + } catch (err) { + if (typeof err === "object" && err !== null && "code" in err && err.code === "ENOENT") return null; + const detail = err instanceof Error && err.message ? `: ${err.message}` : ""; + throw new Error(`Cursor hooks config at ${HOOKS_PATH} could not be read${detail}`); + } + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch { + throw new Error(`Cursor hooks config at ${HOOKS_PATH} is not valid JSON; fix or remove it, then rerun.`); + } + if (parsed === null || typeof parsed !== "object" || Array.isArray(parsed)) { + throw new Error(`Cursor hooks config at ${HOOKS_PATH} must contain a JSON object; fix or remove it, then rerun.`); + } + return parsed as Record; +} + export function stripHooksFromConfig(existing: Record | null): Record | null { if (!existing) return null; const root = existing as { hooks?: Record }; @@ -104,10 +125,13 @@ export function installCursor(): void { throw new Error(`Cursor bundle missing at ${srcBundle}. Run 'npm run build' first.`); } + // Validate before copying payloads. A malformed user hooks file cannot be + // merged safely and must not be replaced with a fresh Hivemind config. + const existing = readHooksConfig(); + ensureDir(PLUGIN_DIR); reportPruned("Cursor", syncDir(srcBundle, join(PLUGIN_DIR, "bundle"))); - const existing = readJson>(HOOKS_PATH); const merged = mergeHooks(existing); // Idempotent (same rationale as codex): skip the rewrite when unchanged so // we don't perturb the hooks.json Cursor/Codex-style trust fingerprints. @@ -125,7 +149,9 @@ export function installCursor(): void { } export function uninstallCursor(): void { - const existing = readJson>(HOOKS_PATH); + // Validation errors propagate to the dispatcher, which reports FAILED. + // Nothing is removed or rewritten before this read succeeds. + const existing = readHooksConfig(); if (!existing) { log(" Cursor no hooks.json to clean"); return; diff --git a/src/cli/install-hermes.ts b/src/cli/install-hermes.ts index 722d7db4..cdb75700 100644 --- a/src/cli/install-hermes.ts +++ b/src/cli/install-hermes.ts @@ -160,20 +160,37 @@ function stripHivemindHooks(existing: Record | undefi return Object.keys(out).length > 0 ? out : undefined; } -function readConfig(): HermesConfig { - if (!existsSync(CONFIG_PATH)) return {}; +function readConfig(): HermesConfig | null { + let raw: string; try { - const raw = readFileSync(CONFIG_PATH, "utf-8"); - const parsed = yaml.load(raw); - if (parsed && typeof parsed === "object" && !Array.isArray(parsed)) { - return parsed as HermesConfig; - } - return {}; + raw = readFileSync(CONFIG_PATH, "utf-8"); + } catch (err) { + if (typeof err === "object" && err !== null && "code" in err && err.code === "ENOENT") return null; + const detail = err instanceof Error && err.message ? `: ${err.message}` : ""; + throw new Error(`Hermes config at ${CONFIG_PATH} could not be read${detail}`); + } + + let parsed: unknown; + let rootNodeIsEmpty = true; + try { + parsed = yaml.load(raw, { + // js-yaml represents both an empty document and an explicit null scalar + // as null. The final parser close event retains the root node metadata, + // so use it to distinguish those cases without reimplementing YAML. + listener: (event, state) => { + if (event !== "close") return; + const node = state as typeof state & { tag?: string | null; anchor?: string | null }; + rootNodeIsEmpty = node.kind == null && node.tag == null && node.anchor == null; + }, + }); } catch { - // Malformed YAML — back off to empty so we don't clobber, but the user - // will see our entry get appended fresh. - return {}; + throw new Error(`Hermes config at ${CONFIG_PATH} is not valid YAML; fix or remove it, then rerun.`); + } + if (parsed && typeof parsed === "object" && !Array.isArray(parsed)) { + return parsed as HermesConfig; } + if ((parsed === undefined || parsed === null) && rootNodeIsEmpty) return {}; + throw new Error(`Hermes config at ${CONFIG_PATH} must contain a YAML mapping; fix or remove it, then rerun.`); } function writeConfig(cfg: HermesConfig): void { @@ -200,6 +217,15 @@ function packagedSkillNames(): string[] { } export function installHermes(): void { + const srcBundle = join(pkgRoot(), "harnesses", "hermes", "bundle"); + if (!existsSync(srcBundle)) { + throw new Error(`Hermes bundle missing at ${srcBundle}. Run 'npm run build' first.`); + } + + // Validate before touching any owned payload directories. Treating a + // malformed config as empty would destroy every user setting on write. + const cfg = readConfig() ?? {}; + // 1. Skills — agent context. hivemind-memory is written inline; everything // else the installer ever put in that dir (an older version's // templates/) is pruned so only the current skill body remains. @@ -218,10 +244,6 @@ export function installHermes(): void { } // 2. Hook bundle — auto-capture via Hermes shell-hooks. - const srcBundle = join(pkgRoot(), "harnesses", "hermes", "bundle"); - if (!existsSync(srcBundle)) { - throw new Error(`Hermes bundle missing at ${srcBundle}. Run 'npm run build' first.`); - } ensureDir(HIVEMIND_DIR); reportPruned("Hermes", syncDir(srcBundle, BUNDLE_DIR)); const pluginNm = join(HIVEMIND_DIR, "node_modules"); @@ -238,7 +260,6 @@ export function installHermes(): void { // Update config.yaml with mcp_servers + hooks + hooks_auto_accept. // Preserves any pre-existing user configuration. - const cfg = readConfig(); if (!cfg.mcp_servers || typeof cfg.mcp_servers !== "object") cfg.mcp_servers = {}; cfg.mcp_servers[SERVER_KEY] = { command: "node", @@ -253,6 +274,11 @@ export function installHermes(): void { } export function uninstallHermes(): void { + // Validate before removing any owned payload. If the config cannot be + // understood, uninstall must fail as one untouched operation rather than + // leave hooks pointing at a bundle that has already been deleted. + const cfg = readConfig(); + for (const dir of [SKILLS_DIR, ...packagedSkillNames().map(n => join(SKILLS_ROOT, n))]) { // A symlink at one of our names is the user's, never something we wrote. if (!existsSync(dir) || isLink(dir)) continue; @@ -265,8 +291,7 @@ export function uninstallHermes(): void { log(` Hermes removed ${HIVEMIND_DIR}`); } - if (existsSync(CONFIG_PATH)) { - const cfg = readConfig(); + if (cfg) { let touched = false; if (cfg.mcp_servers && typeof cfg.mcp_servers === "object" && SERVER_KEY in cfg.mcp_servers) { delete cfg.mcp_servers[SERVER_KEY]; diff --git a/src/user-config.ts b/src/user-config.ts index d66385d5..76a0fa6c 100644 --- a/src/user-config.ts +++ b/src/user-config.ts @@ -26,33 +26,61 @@ export interface UserConfig { let _configPath: () => string = () => process.env.HIVEMIND_CONFIG_PATH ?? join(homedir(), ".deeplake", "config.json"); +type ConfigState = "unloaded" | "missing" | "valid" | "invalid" | "unreadable"; + // In-memory cache so the migration's env-var read and resulting write happen // at most once per process. The file on disk is the source of truth; the // cache only avoids re-parsing JSON on every call. let _cache: UserConfig | null = null; -let _migrated = false; +let _configState: ConfigState = "unloaded"; +let _configReadError: Error | null = null; +let _migrationFallback: boolean | null = null; export function readUserConfig(): UserConfig { if (_cache !== null) return _cache; const path = _configPath(); - if (!existsSync(path)) { + let raw: string; + try { + raw = readFileSync(path, "utf-8"); + } catch (err) { + if (isMissingFileError(err)) { + _configState = "missing"; + _cache = {}; + return _cache; + } + // Passive consumers include capture/startup hooks, so reads remain + // non-throwing. Keep I/O failure distinct from malformed JSON so no + // migration or explicit setter can mistake it for a missing config. + _configState = "unreadable"; + _configReadError = err instanceof Error ? err : new Error(String(err)); _cache = {}; return _cache; } try { - const raw = readFileSync(path, "utf-8"); const parsed = JSON.parse(raw) as unknown; - _cache = isPlainObject(parsed) ? (parsed as UserConfig) : {}; + if (!isPlainObject(parsed)) { + _configState = "invalid"; + _cache = {}; + } else { + _configState = "valid"; + _cache = parsed as UserConfig; + } } catch { - // Corrupt or unreadable — treat as empty, but DON'T overwrite (the user - // may want to fix it by hand). A subsequent write will overwrite. + // Corrupt JSON remains a non-throwing empty view for passive callers, but + // mutation paths refuse to overwrite the bytes the user may want to fix. + _configState = "invalid"; _cache = {}; } return _cache; } +function isMissingFileError(err: unknown): boolean { + return typeof err === "object" && err !== null && "code" in err && err.code === "ENOENT"; +} + export function writeUserConfig(patch: Partial): UserConfig { const current = readUserConfig(); + assertConfigWritable(); const merged = deepMerge(current, patch); const path = _configPath(); const dir = dirname(path); @@ -64,6 +92,16 @@ export function writeUserConfig(patch: Partial): UserConfig { return merged; } +/** + * Validate the current config for a mutation without writing it. Heavy CLI + * operations use this before provisioning or deleting shared resources so a + * malformed or unreadable file cannot fail only after those mutations. + */ +export function assertUserConfigWritable(): void { + readUserConfig(); + assertConfigWritable(); +} + // Reads the embeddings-enabled flag, performing the one-shot env-var // migration if no value has ever been persisted. Returns the final boolean. // @@ -79,24 +117,36 @@ export function getEmbeddingsEnabled(): boolean { if (cfg.embeddings && typeof cfg.embeddings.enabled === "boolean") { return cfg.embeddings.enabled; } - if (_migrated) { - // Migration ran this process but couldn't persist (read-only fs etc.). - // Fall back to the env var directly to avoid spinning the migration on - // every call. Cached for the lifetime of the process. - return migrationValueFromEnv(); - } - _migrated = true; + if (_migrationFallback !== null) return _migrationFallback; + const enabled = migrationValueFromEnv(); + if (_configState === "invalid" || _configState === "unreadable") { + // Hooks must keep running, but an invalid or unreadable file is not an + // empty file: do not migrate over it or infer a merge base from it. + _migrationFallback = enabled; + return enabled; + } try { writeUserConfig({ embeddings: { enabled } }); } catch { - // Persist failed (perms, full disk, etc.) — keep the in-memory cache so - // the rest of the session sees a stable value. - _cache = { ...(cfg ?? {}), embeddings: { ...(cfg?.embeddings ?? {}), enabled } }; + // Persist failed (permissions, full disk, etc.). Preserve the passive + // getter contract and cache only the fallback value, not a guessed config. + _migrationFallback = enabled; } return enabled; } +function assertConfigWritable(): void { + const path = _configPath(); + if (_configState === "invalid") { + throw new Error(`Hivemind user config at ${path} is not valid JSON; fix or remove it, then rerun.`); + } + if (_configState === "unreadable") { + const detail = _configReadError?.message ? `: ${_configReadError.message}` : ""; + throw new Error(`Hivemind user config at ${path} could not be read${detail}`); + } +} + function migrationValueFromEnv(): boolean { const raw = process.env.HIVEMIND_EMBEDDINGS; if (raw === undefined) return false; @@ -146,12 +196,16 @@ function deepMerge(base: UserConfig, patch: Partial): UserConfig { export function _setConfigPathForTesting(fn: () => string): void { _configPath = fn; _cache = null; - _migrated = false; + _configState = "unloaded"; + _configReadError = null; + _migrationFallback = null; } export function _resetUserConfigForTesting(): void { _configPath = () => process.env.HIVEMIND_CONFIG_PATH ?? join(homedir(), ".deeplake", "config.json"); _cache = null; - _migrated = false; + _configState = "unloaded"; + _configReadError = null; + _migrationFallback = null; } diff --git a/tests/claude-code/user-config-io-failure.test.ts b/tests/claude-code/user-config-io-failure.test.ts new file mode 100644 index 00000000..99604254 --- /dev/null +++ b/tests/claude-code/user-config-io-failure.test.ts @@ -0,0 +1,62 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +const fsFault = vi.hoisted(() => ({ + path: "", + error: null as NodeJS.ErrnoException | null, + writeFileSync: vi.fn(), + renameSync: vi.fn(), +})); + +vi.mock("node:fs", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + readFileSync: (path: unknown, ...args: unknown[]) => { + if (path === fsFault.path && fsFault.error) throw fsFault.error; + return (actual.readFileSync as (...inner: unknown[]) => unknown)(path, ...args); + }, + writeFileSync: (...args: unknown[]) => { + fsFault.writeFileSync(...args); + return (actual.writeFileSync as (...inner: unknown[]) => unknown)(...args); + }, + renameSync: (...args: unknown[]) => { + fsFault.renameSync(...args); + return (actual.renameSync as (...inner: unknown[]) => unknown)(...args); + }, + }; +}); + +import { + _resetUserConfigForTesting, + _setConfigPathForTesting, + getEmbeddingsEnabled, + readUserConfig, + setEmbeddingsEnabled, +} from "../../src/user-config.js"; + +const originalEnv = process.env.HIVEMIND_EMBEDDINGS; + +beforeEach(() => { + fsFault.path = "/tmp/hivemind-injected-io-error/config.json"; + fsFault.error = Object.assign(new Error("injected config read failure"), { code: "EIO" }); + fsFault.writeFileSync.mockClear(); + fsFault.renameSync.mockClear(); + _setConfigPathForTesting(() => fsFault.path); + process.env.HIVEMIND_EMBEDDINGS = "true"; +}); + +afterEach(() => { + _resetUserConfigForTesting(); + if (originalEnv === undefined) delete process.env.HIVEMIND_EMBEDDINGS; + else process.env.HIVEMIND_EMBEDDINGS = originalEnv; +}); + +describe("user config I/O failures", () => { + it("keeps passive getters safe but never migrates or explicitly writes over an unreadable config", () => { + expect(readUserConfig()).toEqual({}); + expect(getEmbeddingsEnabled()).toBe(true); + expect(() => setEmbeddingsEnabled(false)).toThrow(/could not be read.*injected config read failure/); + expect(fsFault.writeFileSync).not.toHaveBeenCalled(); + expect(fsFault.renameSync).not.toHaveBeenCalled(); + }); +}); diff --git a/tests/claude-code/user-config.test.ts b/tests/claude-code/user-config.test.ts index 5931852e..e49b1957 100644 --- a/tests/claude-code/user-config.test.ts +++ b/tests/claude-code/user-config.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, beforeEach, afterEach } from "vitest"; -import { mkdtempSync, rmSync, existsSync, readFileSync, writeFileSync } from "node:fs"; +import { mkdirSync, mkdtempSync, rmSync, existsSync, readFileSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { @@ -60,6 +60,25 @@ describe("readUserConfig", () => { expect(readUserConfig()).toEqual({}); }); + it("does not overwrite malformed JSON when embeddings migration runs", () => { + const original = "{ not json"; + writeFileSync(configPath, original, "utf-8"); + _resetUserConfigForTesting(); + _setConfigPathForTesting(() => configPath); + expect(getEmbeddingsEnabled()).toBe(false); + expect(readFileSync(configPath, "utf-8")).toBe(original); + }); + + it("keeps passive reads non-throwing when the config cannot be read, without attempting migration", () => { + mkdirSync(configPath); + process.env.HIVEMIND_EMBEDDINGS = "true"; + + expect(readUserConfig()).toEqual({}); + expect(getEmbeddingsEnabled()).toBe(true); + expect(() => setEmbeddingsEnabled(false)).toThrow(/could not be read/); + expect(existsSync(configPath)).toBe(true); + }); + it("caches the parsed config across calls (single file read per process)", () => { writeFileSync(configPath, JSON.stringify({ embeddings: { enabled: false } }), "utf-8"); _resetUserConfigForTesting(); @@ -169,4 +188,13 @@ describe("getEmbeddingsEnabled — migration from HIVEMIND_EMBEDDINGS", () => { const written = JSON.parse(readFileSync(configPath, "utf-8")); expect(written).toEqual({ embeddings: { enabled: true } }); }); + + it("refuses explicit writes over malformed config", () => { + const original = "{ not json"; + writeFileSync(configPath, original, "utf-8"); + _resetUserConfigForTesting(); + _setConfigPathForTesting(() => configPath); + expect(() => setEmbeddingsEnabled(true)).toThrow(/not valid JSON/); + expect(readFileSync(configPath, "utf-8")).toBe(original); + }); }); diff --git a/tests/cli/cli-embeddings-config-preflight.test.ts b/tests/cli/cli-embeddings-config-preflight.test.ts new file mode 100644 index 00000000..64d32af4 --- /dev/null +++ b/tests/cli/cli-embeddings-config-preflight.test.ts @@ -0,0 +1,155 @@ +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; +import { execFileSync } from "node:child_process"; +import { existsSync, lstatSync, mkdirSync, mkdtempSync, readFileSync, readlinkSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { setFakeHome, clearFakeHome } from "../shared/fake-home.js"; + +const fsFault = vi.hoisted(() => ({ + path: "", + error: null as NodeJS.ErrnoException | null, +})); +const graphDeps = vi.hoisted(() => ({ ensureGraphDeps: vi.fn() })); + +vi.mock("node:child_process", async (importOriginal) => ({ + ...(await importOriginal()), + execFileSync: vi.fn(), +})); + +vi.mock("node:fs", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + readFileSync: (path: unknown, ...args: unknown[]) => { + if (path === fsFault.path && fsFault.error) throw fsFault.error; + return (actual.readFileSync as (...inner: unknown[]) => unknown)(path, ...args); + }, + }; +}); + +vi.mock("../../src/cli/graph-deps.js", () => graphDeps); + +const HOME_DIR = mkdtempSync(join(tmpdir(), "emb-config-preflight-home-")); +let mod: typeof import("../../src/cli/embeddings.js"); +let cfg: typeof import("../../src/user-config.js"); +let processKill: ReturnType; + +beforeAll(async () => { + setFakeHome(HOME_DIR); + mod = await import("../../src/cli/embeddings.js"); + cfg = await import("../../src/user-config.js"); +}); + +beforeEach(() => { + for (const sub of [".hivemind", ".deeplake", ".codex", ".cursor", ".hermes", ".claude"]) { + rmSync(join(HOME_DIR, sub), { recursive: true, force: true }); + } + cfg._resetUserConfigForTesting(); + cfg._setConfigPathForTesting(() => join(HOME_DIR, ".deeplake", "config.json")); + mkdirSync(join(HOME_DIR, ".deeplake"), { recursive: true }); + fsFault.path = ""; + fsFault.error = null; + graphDeps.ensureGraphDeps.mockClear(); + vi.mocked(execFileSync).mockClear(); + processKill = vi.spyOn(process, "kill").mockImplementation(() => true); +}); + +afterEach(() => { + processKill.mockRestore(); +}); + +afterAll(() => { + cfg._resetUserConfigForTesting(); + clearFakeHome(); + rmSync(HOME_DIR, { recursive: true, force: true }); + vi.resetModules(); +}); + +function seedFixture(configBytes: string): { configPath: string; linkPath: string; sharedPayloadPath: string; linkInode: number } { + const configPath = join(HOME_DIR, ".deeplake", "config.json"); + writeFileSync(configPath, configBytes); + const sharedPayloadPath = join(mod.SHARED_DIR, "fixture-payload.bin"); + mkdirSync(join(mod.SHARED_NODE_MODULES, mod.TRANSFORMERS_PKG), { recursive: true }); + writeFileSync(sharedPayloadPath, "shared payload stays byte-for-byte"); + const pluginDir = join(HOME_DIR, ".codex", "hivemind"); + mkdirSync(join(pluginDir, "bundle"), { recursive: true }); + const linkPath = join(pluginDir, "node_modules"); + symlinkSync(mod.SHARED_NODE_MODULES, linkPath); + return { configPath, linkPath, sharedPayloadPath, linkInode: lstatSync(linkPath).ino }; +} + +describe("heavy embeddings config preflight", () => { + it.each([ + ["install", "{ malformed"], + ["uninstall", "{ malformed"], + ])("rejects malformed config before %s mutation", (operation, configBytes) => { + const fixture = seedFixture(configBytes); + + expect(() => operation === "install" + ? mod.installEmbeddings() + : mod.uninstallEmbeddings({ prune: true })).toThrow(/not valid JSON/); + expect(readFileSync(fixture.configPath, "utf-8")).toBe(configBytes); + expect(readFileSync(fixture.sharedPayloadPath, "utf-8")).toBe("shared payload stays byte-for-byte"); + expect(existsSync(mod.SHARED_DIR)).toBe(true); + expect(lstatSync(fixture.linkPath).isSymbolicLink()).toBe(true); + expect(readlinkSync(fixture.linkPath)).toBe(mod.SHARED_NODE_MODULES); + expect(lstatSync(fixture.linkPath).ino).toBe(fixture.linkInode); + expect(graphDeps.ensureGraphDeps).not.toHaveBeenCalled(); + expect(execFileSync).not.toHaveBeenCalled(); + expect(processKill).not.toHaveBeenCalled(); + }); + + it.each(["install", "uninstall"])("rejects injected config read errors before %s mutation", (operation) => { + const fixture = seedFixture(JSON.stringify({ embeddings: { enabled: true } })); + fsFault.path = fixture.configPath; + fsFault.error = Object.assign(new Error("injected config read failure"), { code: "EIO" }); + + expect(() => operation === "install" + ? mod.installEmbeddings() + : mod.uninstallEmbeddings({ prune: true })).toThrow(/could not be read.*injected config read failure/); + fsFault.error = null; + expect(readFileSync(fixture.configPath, "utf-8")).toBe(JSON.stringify({ embeddings: { enabled: true } })); + expect(readFileSync(fixture.sharedPayloadPath, "utf-8")).toBe("shared payload stays byte-for-byte"); + expect(existsSync(mod.SHARED_DIR)).toBe(true); + expect(lstatSync(fixture.linkPath).isSymbolicLink()).toBe(true); + expect(readlinkSync(fixture.linkPath)).toBe(mod.SHARED_NODE_MODULES); + expect(lstatSync(fixture.linkPath).ino).toBe(fixture.linkInode); + expect(graphDeps.ensureGraphDeps).not.toHaveBeenCalled(); + expect(execFileSync).not.toHaveBeenCalled(); + expect(processKill).not.toHaveBeenCalled(); + }); + + it("allows install with a valid config and existing payload/link", () => { + const config = { docs: { llmAgent: "codex" }, embeddings: { enabled: false } }; + const fixture = seedFixture(JSON.stringify(config)); + + mod.installEmbeddings(); + + expect(JSON.parse(readFileSync(fixture.configPath, "utf-8"))).toEqual({ + docs: { llmAgent: "codex" }, + embeddings: { enabled: true }, + }); + expect(readFileSync(fixture.sharedPayloadPath, "utf-8")).toBe("shared payload stays byte-for-byte"); + expect(lstatSync(fixture.linkPath).isSymbolicLink()).toBe(true); + expect(readlinkSync(fixture.linkPath)).toBe(mod.SHARED_NODE_MODULES); + expect(graphDeps.ensureGraphDeps).toHaveBeenCalledTimes(1); + expect(execFileSync).not.toHaveBeenCalled(); + }); + + it("allows prune-uninstall with a valid config and existing payload/link", () => { + const config = { docs: { llmAgent: "codex" }, embeddings: { enabled: true } }; + const fixture = seedFixture(JSON.stringify(config)); + + mod.uninstallEmbeddings({ prune: true }); + + expect(JSON.parse(readFileSync(fixture.configPath, "utf-8"))).toEqual({ + docs: { llmAgent: "codex" }, + embeddings: { enabled: false }, + }); + expect(existsSync(fixture.linkPath)).toBe(false); + expect(existsSync(fixture.sharedPayloadPath)).toBe(false); + expect(existsSync(mod.SHARED_DIR)).toBe(false); + expect(graphDeps.ensureGraphDeps).not.toHaveBeenCalled(); + expect(execFileSync).not.toHaveBeenCalled(); + }); +}); diff --git a/tests/cli/cli-install-codex-fs.test.ts b/tests/cli/cli-install-codex-fs.test.ts index bed6299f..041c617d 100644 --- a/tests/cli/cli-install-codex-fs.test.ts +++ b/tests/cli/cli-install-codex-fs.test.ts @@ -59,12 +59,28 @@ beforeEach(() => { afterEach(() => { rmSync(tmpRoot, { recursive: true, force: true }); clearFakeHome(); + vi.doUnmock("node:fs"); vi.restoreAllMocks(); vi.resetModules(); }); -async function importInstaller(): Promise { +async function importInstaller(configReadError?: NodeJS.ErrnoException): Promise { vi.resetModules(); + if (configReadError) { + const configPath = join(tmpHome, ".codex", "hooks.json"); + vi.doMock("node:fs", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + readFileSync: (path: unknown, ...args: unknown[]) => { + if (path === configPath) throw configReadError; + return (actual.readFileSync as (...inner: unknown[]) => unknown)(path, ...args); + }, + }; + }); + } else { + vi.doUnmock("node:fs"); + } vi.doMock("../../src/cli/util.js", async (importOriginal) => { const actual = await importOriginal(); return { ...actual, pkgRoot: () => tmpPkg }; @@ -266,12 +282,13 @@ describe("installCodex — happy path", () => { expect(hooks.version).toBe(7); }); - it("re-install over a malformed hooks.json does not throw and writes a fresh one", async () => { - writeFileSync(join(tmpHome, ".codex", "hooks.json"), "{ not json"); + it("refuses a malformed hooks.json without overwriting it or touching payloads", async () => { + const hooksPath = join(tmpHome, ".codex", "hooks.json"); + writeFileSync(hooksPath, "{ not json"); const { installCodex } = await importInstaller(); - expect(() => installCodex()).not.toThrow(); - const hooks = JSON.parse(readFileSync(join(tmpHome, ".codex", "hooks.json"), "utf-8")); - expect(hooks.hooks.SessionStart).toBeDefined(); + expect(() => installCodex()).toThrow(/not valid JSON/); + expect(readFileSync(hooksPath, "utf-8")).toBe("{ not json"); + expect(existsSync(join(tmpHome, ".codex", "hivemind"))).toBe(false); }); it("warns and skips the symlink (without throwing) when the skill source is missing", async () => { @@ -493,14 +510,27 @@ describe("uninstallCodex", () => { expect(after.hooks.SessionStart ?? []).toHaveLength(0); }); - it("uninstall on a malformed hooks.json deletes the file rather than crashing", async () => { - // Lock the catch path inside uninstallCodex that handles unparseable - // JSON: we'd rather drop the file than guess at intent (the user can - // re-install cleanly). + it("uninstall on a malformed hooks.json reports failure and leaves the file untouched", async () => { const { uninstallCodex } = await importInstaller(); const hooksPath = join(tmpHome, ".codex", "hooks.json"); writeFileSync(hooksPath, "{ this is not valid json"); - expect(() => uninstallCodex()).not.toThrow(); - expect(existsSync(hooksPath)).toBe(false); + expect(() => uninstallCodex()).toThrow(/not valid JSON/); + expect(readFileSync(hooksPath, "utf-8")).toBe("{ this is not valid json"); + }); + + it("validates hooks.json read errors before removing other Codex integration files", async () => { + const { installCodex } = await importInstaller(); + installCodex(); + const hooksPath = join(tmpHome, ".codex", "hooks.json"); + const agentsPath = join(tmpHome, ".codex", "AGENTS.md"); + const originalHooks = readFileSync(hooksPath, "utf-8"); + const originalAgents = readFileSync(agentsPath, "utf-8"); + const readError = Object.assign(new Error("injected hooks read failure"), { code: "EACCES" }); + const { uninstallCodex } = await importInstaller(readError); + + expect(() => uninstallCodex()).toThrow(/could not be read/); + expect(readFileSync(hooksPath, "utf-8")).toBe(originalHooks); + expect(readFileSync(agentsPath, "utf-8")).toBe(originalAgents); + expect(existsSync(join(tmpHome, ".agents", "skills", "hivemind-memory"))).toBe(true); }); }); diff --git a/tests/cli/cli-install-cursor-fs.test.ts b/tests/cli/cli-install-cursor-fs.test.ts index fda002da..8d90942a 100644 --- a/tests/cli/cli-install-cursor-fs.test.ts +++ b/tests/cli/cli-install-cursor-fs.test.ts @@ -118,6 +118,23 @@ describe("installCursor", () => { expect(() => installCursor()).toThrow(/Cursor bundle missing/); }); + it("refuses a malformed hooks.json without overwriting it or touching payloads", async () => { + const hooksPath = join(tmpHome, ".cursor", "hooks.json"); + writeFileSync(hooksPath, "{ not json"); + const { installCursor } = await importInstaller(); + expect(() => installCursor()).toThrow(/not valid JSON/); + expect(readFileSync(hooksPath, "utf-8")).toBe("{ not json"); + expect(existsSync(join(tmpHome, ".cursor", "hivemind"))).toBe(false); + }); + + it("uninstall on a malformed hooks.json reports failure and leaves the file untouched", async () => { + const hooksPath = join(tmpHome, ".cursor", "hooks.json"); + writeFileSync(hooksPath, "{ not json"); + const { uninstallCursor } = await importInstaller(); + expect(() => uninstallCursor()).toThrow(/not valid JSON/); + expect(readFileSync(hooksPath, "utf-8")).toBe("{ not json"); + }); + it("creates embed-deps symlink when ~/.hivemind/embed-deps/node_modules exists", async () => { const embedDepsNm = join(tmpHome, ".hivemind", "embed-deps", "node_modules"); mkdirSync(embedDepsNm, { recursive: true }); diff --git a/tests/cli/cli-install-hermes.test.ts b/tests/cli/cli-install-hermes.test.ts index d2ce93ba..015ae4fe 100644 --- a/tests/cli/cli-install-hermes.test.ts +++ b/tests/cli/cli-install-hermes.test.ts @@ -1,5 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { mkdirSync, rmSync, writeFileSync, readFileSync, existsSync } from "node:fs"; +import { chmodSync, mkdirSync, rmSync, writeFileSync, readFileSync, existsSync, statSync } from "node:fs"; import { join } from "node:path"; import { tmpdir } from "node:os"; import * as yaml from "js-yaml"; @@ -44,12 +44,28 @@ beforeEach(() => { afterEach(() => { rmSync(tmpRoot, { recursive: true, force: true }); clearFakeHome(); + vi.doUnmock("node:fs"); vi.restoreAllMocks(); vi.resetModules(); }); -async function importInstaller(): Promise { +async function importInstaller(configReadError?: NodeJS.ErrnoException): Promise { vi.resetModules(); + if (configReadError) { + const configPath = join(tmpHome, ".hermes", "config.yaml"); + vi.doMock("node:fs", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + readFileSync: (path: unknown, ...args: unknown[]) => { + if (path === configPath) throw configReadError; + return (actual.readFileSync as (...inner: unknown[]) => unknown)(path, ...args); + }, + }; + }); + } else { + vi.doUnmock("node:fs"); + } vi.doMock("../../src/cli/util.js", async (importOriginal) => { const actual = await importOriginal(); return { ...actual, pkgRoot: () => tmpPkg }; @@ -119,12 +135,56 @@ describe("installHermes — cold install", () => { expect(cfg.preserved_field).toBe("stay"); }); - it("re-install over a malformed config.yaml does not throw and lands a fresh entry", async () => { + it.each([ + "", + " \n\t", + "# user comment only\n # another comment\n", + "---\n", + "--- # comment\n", + "...\n", + "---\n# comment before the document end\n... # end\n", + "\ufeff---\n", + "%YAML 1.2\n---\n...\n", + "%TAG !e! tag:example.com,2026:\n---\n...\n", + ])( + "accepts an empty YAML document without treating it as malformed (%j)", + async (raw) => { + mkdirSync(join(tmpHome, ".hermes"), { recursive: true }); + writeFileSync(join(tmpHome, ".hermes", "config.yaml"), raw); + + const { installHermes } = await importInstaller(); + expect(() => installHermes()).not.toThrow(); + expect(readConfig().mcp_servers.hivemind.command).toBe("node"); + }, + ); + + it.each(["plain scalar\n", "- array item\n", "null\n", "~\n", "!!null\n", "&empty\n"])( + "rejects a non-mapping YAML root without touching payloads (%j)", + async (raw) => { + mkdirSync(join(tmpHome, ".hermes"), { recursive: true }); + const configPath = join(tmpHome, ".hermes", "config.yaml"); + writeFileSync(configPath, raw); + + const { installHermes } = await importInstaller(); + expect(() => installHermes()).toThrow(/must contain a YAML mapping/); + expect(readFileSync(configPath, "utf-8")).toBe(raw); + expect(existsSync(join(tmpHome, ".hermes", "hivemind"))).toBe(false); + expect(existsSync(join(tmpHome, ".hivemind", "mcp"))).toBe(false); + }, + ); + + it("refuses a malformed config.yaml without overwriting it or touching payloads", async () => { mkdirSync(join(tmpHome, ".hermes"), { recursive: true }); - writeFileSync(join(tmpHome, ".hermes", "config.yaml"), "::: not yaml :::"); + const configPath = join(tmpHome, ".hermes", "config.yaml"); + writeFileSync(configPath, "::: not yaml :::"); + chmodSync(configPath, 0o640); + const originalMode = statSync(configPath).mode & 0o777; const { installHermes } = await importInstaller(); - expect(() => installHermes()).not.toThrow(); - expect(readConfig().mcp_servers.hivemind).toBeDefined(); + expect(() => installHermes()).toThrow(/not valid YAML/); + expect(readFileSync(configPath, "utf-8")).toBe("::: not yaml :::"); + expect(statSync(configPath).mode & 0o777).toBe(originalMode); + expect(existsSync(join(tmpHome, ".hermes", "hivemind"))).toBe(false); + expect(existsSync(join(tmpHome, ".hivemind", "mcp"))).toBe(false); }); it("re-install replaces stale hivemind hooks (no duplication after N re-runs)", async () => { @@ -193,6 +253,36 @@ describe("uninstallHermes", () => { expect(cfg.mcp_servers.hivemind).toBeUndefined(); }); + it("validates malformed config before removing any payload and propagates failure", async () => { + const { installHermes, uninstallHermes } = await importInstaller(); + installHermes(); + const configPath = join(tmpHome, ".hermes", "config.yaml"); + const original = "hooks: [unterminated"; + writeFileSync(configPath, original); + chmodSync(configPath, 0o640); + const originalMode = statSync(configPath).mode & 0o777; + + expect(() => uninstallHermes()).toThrow(/not valid YAML/); + expect(readFileSync(configPath, "utf-8")).toBe(original); + expect(statSync(configPath).mode & 0o777).toBe(originalMode); + expect(existsSync(join(tmpHome, ".hermes", "skills", "hivemind-memory", "SKILL.md"))).toBe(true); + expect(existsSync(join(tmpHome, ".hermes", "hivemind", "bundle", "capture.js"))).toBe(true); + }); + + it("does not remove payload when config reading fails at the filesystem boundary", async () => { + const { installHermes } = await importInstaller(); + installHermes(); + const configPath = join(tmpHome, ".hermes", "config.yaml"); + const original = readFileSync(configPath, "utf-8"); + const readError = Object.assign(new Error("injected config read failure"), { code: "EIO" }); + const { uninstallHermes } = await importInstaller(readError); + + expect(() => uninstallHermes()).toThrow(/could not be read/); + expect(readFileSync(configPath, "utf-8")).toBe(original); + expect(existsSync(join(tmpHome, ".hermes", "skills", "hivemind-memory", "SKILL.md"))).toBe(true); + expect(existsSync(join(tmpHome, ".hermes", "hivemind", "bundle", "capture.js"))).toBe(true); + }); + it("is a no-op (no throw) when nothing has been installed", async () => { const { uninstallHermes } = await importInstaller(); expect(() => uninstallHermes()).not.toThrow();