Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 34 additions & 9 deletions src/lib/config-io.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand All @@ -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", () => {
Expand All @@ -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;
Expand All @@ -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");
});
});
96 changes: 77 additions & 19 deletions src/lib/config-io.ts
Original file line number Diff line number Diff line change
@@ -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");
Comment on lines +10 to +22
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Remediation still assumes sudo exists.

Issue #692 explicitly includes environments where sudo is unavailable, but both suggested fixes here still require it. In those installs the new ConfigPermissionError is still not actionable. Please add a non-sudo fallback, e.g. recreating config under a user-writable HOME or config directory.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/lib/config-io.ts` around lines 29 - 41, The current buildRemediation()
message assumes sudo is available; update it to include non-sudo fallback
actions so environments without sudo get actionable guidance. In the
buildRemediation() function (reference: nemoclawDir and HOME usage) add
alternative suggestions such as recreating the config under the current user's
home (e.g., remove or move the directory if writable), instructing to remove the
directory without sudo when the user owns it (e.g., rm -rf $HOME/.nemoclaw), and
advising to relocate or initialize config in a user-writable path (for example
creating a new config under $HOME or specifying an alternative CONFIG_HOME), so
the error message covers both sudo and non-sudo environments. Ensure the text
clearly distinguishes when sudo is required vs when the non-sudo command
applies.

}

function isPermissionError(error: unknown): error is NodeJS.ErrnoException {
Expand All @@ -26,34 +31,87 @@ 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;
}
}

export function readConfigFile<T>(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 {
Expand All @@ -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;
}
Expand Down
4 changes: 2 additions & 2 deletions src/lib/registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -30,7 +30,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 {
Expand Down
Loading