diff --git a/src/lib/config-io.test.ts b/src/lib/config-io.test.ts index 628a6a6f6..30eadde19 100644 --- a/src/lib/config-io.test.ts +++ b/src/lib/config-io.test.ts @@ -28,10 +28,20 @@ afterEach(() => { }); describe("config-io", () => { - it("creates config directories recursively", () => { + it("creates config directories recursively with mode 0o700", () => { const dir = path.join(makeTempDir(), "a", "b", "c"); ensureConfigDir(dir); expect(fs.existsSync(dir)).toBe(true); + expect(fs.statSync(dir).mode & 0o777).toBe(0o700); + }); + + it("tightens pre-existing weak directory permissions to 0o700", () => { + const dir = path.join(makeTempDir(), "config"); + fs.mkdirSync(dir, { mode: 0o755 }); + + ensureConfigDir(dir); + + expect(fs.statSync(dir).mode & 0o777).toBe(0o700); }); it("returns the fallback when the config file is missing", () => { @@ -49,11 +59,13 @@ describe("config-io", () => { it("writes and reads JSON atomically", () => { const dir = makeTempDir(); const file = path.join(dir, "config.json"); - writeConfigFile(file, { token: "abc", nested: { enabled: true } }); + const data = { token: "abc", nested: { enabled: true } }; - expect(readConfigFile(file, null)).toEqual({ token: "abc", nested: { enabled: true } }); - const leftovers = fs.readdirSync(dir).filter((name) => name.includes(".tmp.")); - expect(leftovers).toEqual([]); + writeConfigFile(file, data); + + expect(readConfigFile(file, null)).toEqual(data); + expect(fs.statSync(file).mode & 0o777).toBe(0o600); + expect(fs.readdirSync(dir).filter((name) => name.includes(".tmp."))).toEqual([]); }); it("cleans up temp files when rename fails", () => { @@ -68,11 +80,10 @@ describe("config-io", () => { } finally { fs.renameSync = originalRename; } - const leftovers = fs.readdirSync(dir).filter((name) => name.includes(".tmp.")); - expect(leftovers).toEqual([]); + expect(fs.readdirSync(dir).filter((name) => name.includes(".tmp."))).toEqual([]); }); - it("wraps permission errors with a user-facing error", () => { + it("wraps permission errors with remediation guidance", () => { const dir = makeTempDir(); const file = path.join(dir, "config.json"); const originalWrite = fs.writeFileSync; @@ -81,9 +92,23 @@ describe("config-io", () => { }; try { expect(() => writeConfigFile(file, { ok: true })).toThrow(ConfigPermissionError); - expect(() => writeConfigFile(file, { ok: true })).toThrow(/HOME points to a user-owned directory/); + expect(() => writeConfigFile(file, { ok: true })).toThrow(/sudo chown/); } finally { fs.writeFileSync = originalWrite; } }); + + it("supports both rich and legacy constructor forms", () => { + const rich = new ConfigPermissionError("test error", "/some/path"); + expect(rich.name).toBe("ConfigPermissionError"); + expect(rich.code).toBe("EACCES"); + expect(rich.configPath).toBe("/some/path"); + expect(rich.filePath).toBe("/some/path"); + expect(rich.message).toContain("test error"); + expect(rich.remediation).toContain("sudo chown"); + + const legacy = new ConfigPermissionError("/other/path", "write"); + expect(legacy.filePath).toBe("/other/path"); + expect(legacy.message).toContain("Cannot write config file"); + }); }); diff --git a/src/lib/config-io.ts b/src/lib/config-io.ts index 7e2b105c5..301d5c2c8 100644 --- a/src/lib/config-io.ts +++ b/src/lib/config-io.ts @@ -1,20 +1,25 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 +// +// Safe config file I/O with permission-aware errors and atomic writes. import fs from "node:fs"; +import os from "node:os"; import path from "node:path"; -export class ConfigPermissionError extends Error { - filePath: string; - - constructor(filePath: string, action: "read" | "write" | "create directory") { - super( - `Cannot ${action} config file at ${filePath}. ` + - "Check that HOME points to a user-owned directory and that ~/.nemoclaw is writable.", - ); - this.name = "ConfigPermissionError"; - this.filePath = filePath; - } +function buildRemediation(): string { + const home = process.env.HOME || os.homedir(); + const nemoclawDir = path.join(home, ".nemoclaw"); + return [ + " To fix, run one of:", + "", + ` sudo chown -R $(whoami) ${nemoclawDir}`, + " # or, if the directory was created by another user:", + ` sudo rm -rf ${nemoclawDir} && nemoclaw onboard`, + "", + " This usually happens when NemoClaw was first run with sudo", + " or the config directory was created by a different user.", + ].join("\n"); } function isPermissionError(error: unknown): error is NodeJS.ErrnoException { @@ -26,12 +31,65 @@ function isPermissionError(error: unknown): error is NodeJS.ErrnoException { ); } +export class ConfigPermissionError extends Error { + code = "EACCES"; + configPath: string; + filePath: string; + remediation: string; + + constructor(filePath: string, action: "read" | "write" | "create directory"); + constructor(message: string, configPath: string, cause?: Error); + constructor(messageOrPath: string, configPathOrAction: string, cause?: Error) { + const action = + configPathOrAction === "read" || + configPathOrAction === "write" || + configPathOrAction === "create directory" + ? configPathOrAction + : null; + + const configPath = action ? messageOrPath : configPathOrAction; + const message = action + ? action === "create directory" + ? `Cannot create config directory: ${configPath}` + : `Cannot ${action} config file: ${configPath}` + : messageOrPath; + + const remediation = buildRemediation(); + super(`${message}\n\n${remediation}`); + this.name = "ConfigPermissionError"; + this.configPath = configPath; + this.filePath = configPath; + this.remediation = remediation; + if (cause) { + this.cause = cause; + } + } +} + export function ensureConfigDir(dirPath: string): void { try { fs.mkdirSync(dirPath, { recursive: true, mode: 0o700 }); + + const stat = fs.statSync(dirPath); + if ((stat.mode & 0o077) !== 0) { + fs.chmodSync(dirPath, 0o700); + } + } catch (error) { + if (isPermissionError(error)) { + throw new ConfigPermissionError(`Cannot create config directory: ${dirPath}`, dirPath, error as Error); + } + throw error; + } + + try { + fs.accessSync(dirPath, fs.constants.W_OK); } catch (error) { if (isPermissionError(error)) { - throw new ConfigPermissionError(dirPath, "create directory"); + throw new ConfigPermissionError( + `Config directory exists but is not writable: ${dirPath}`, + dirPath, + error as Error, + ); } throw error; } @@ -39,21 +97,21 @@ export function ensureConfigDir(dirPath: string): void { export function readConfigFile(filePath: string, fallback: T): T { try { - if (!fs.existsSync(filePath)) { - return fallback; - } return JSON.parse(fs.readFileSync(filePath, "utf-8")) as T; } catch (error) { if (isPermissionError(error)) { - throw new ConfigPermissionError(filePath, "read"); + throw new ConfigPermissionError(`Cannot read config file: ${filePath}`, filePath, error as Error); + } + if ((error as NodeJS.ErrnoException).code === "ENOENT") { + return fallback; } return fallback; } } export function writeConfigFile(filePath: string, data: unknown): void { - const dir = path.dirname(filePath); - ensureConfigDir(dir); + const dirPath = path.dirname(filePath); + ensureConfigDir(dirPath); const tmpFile = `${filePath}.tmp.${process.pid}`; try { @@ -66,7 +124,7 @@ export function writeConfigFile(filePath: string, data: unknown): void { /* best effort */ } if (isPermissionError(error)) { - throw new ConfigPermissionError(filePath, "write"); + throw new ConfigPermissionError(`Cannot write config file: ${filePath}`, filePath, error as Error); } throw error; } diff --git a/src/lib/registry.ts b/src/lib/registry.ts index 4969a9697..30c188af8 100644 --- a/src/lib/registry.ts +++ b/src/lib/registry.ts @@ -4,7 +4,7 @@ import fs from "node:fs"; import path from "node:path"; -import { readConfigFile, writeConfigFile } from "./config-io"; +import { ensureConfigDir, readConfigFile, writeConfigFile } from "./config-io"; export interface SandboxEntry { name: string; @@ -32,7 +32,7 @@ export const LOCK_MAX_RETRIES = 120; /** Acquire an advisory lock using mkdir (atomic on POSIX). */ export function acquireLock(): void { - fs.mkdirSync(path.dirname(REGISTRY_FILE), { recursive: true, mode: 0o700 }); + ensureConfigDir(path.dirname(REGISTRY_FILE)); const sleepBuf = new Int32Array(new SharedArrayBuffer(4)); for (let i = 0; i < LOCK_MAX_RETRIES; i++) { try {