diff --git a/nemoclaw-blueprint/scripts/ws-proxy-fix.js b/nemoclaw-blueprint/scripts/ws-proxy-fix.js index 247d301277..1b9a962675 100644 --- a/nemoclaw-blueprint/scripts/ws-proxy-fix.js +++ b/nemoclaw-blueprint/scripts/ws-proxy-fix.js @@ -174,6 +174,9 @@ const _PATCHED = Symbol.for("nemoclaw.wsProxyFix"); host = opts.host.replace(/:\d+$/, ""); } if (isDiscordWsUpgrade(host, opts.headers)) { + // Guard: if isDiscordWsUpgrade matched but host resolved to + // undefined, we cannot construct a CONNECT tunnel (no target). + // Fall through to the original https.request unchanged. if (!host) { return callOriginalRequest(input, options, callback); } diff --git a/nemoclaw-blueprint/scripts/ws-proxy-fix.ts b/nemoclaw-blueprint/scripts/ws-proxy-fix.ts index 730e52cf86..f1451aa401 100644 --- a/nemoclaw-blueprint/scripts/ws-proxy-fix.ts +++ b/nemoclaw-blueprint/scripts/ws-proxy-fix.ts @@ -224,6 +224,11 @@ interface ReqOpts extends https.RequestOptions { host = opts.host.replace(/:\d+$/, ""); } if (isDiscordWsUpgrade(host, opts.headers)) { + // Guard: if isDiscordWsUpgrade matched but host resolved to + // undefined, we cannot construct a CONNECT tunnel (no target). + // Fall through to the original https.request unchanged. Before + // PR #2422 this path would have attempted the tunnel with an + // undefined host, which would fail in createTunnelAgent anyway. if (!host) { return callOriginalRequest(input, options, callback); } diff --git a/src/lib/agent-onboard.ts b/src/lib/agent-onboard.ts index e465540ec5..e632ebfdea 100644 --- a/src/lib/agent-onboard.ts +++ b/src/lib/agent-onboard.ts @@ -14,10 +14,7 @@ import { loadAgent, resolveAgentName, type AgentDefinition } from "./agent-defs" import { getProviderSelectionConfig } from "./inference-config"; import * as onboardSession from "./onboard-session"; import { sleepSeconds } from "./wait"; - -type LooseScalar = string | number | boolean | null | undefined; -type LooseValue = LooseScalar | LooseObject | LooseValue[]; -type LooseObject = { [key: string]: LooseValue }; +import type { JsonValue as LooseValue, JsonObject as LooseObject } from "./json-types"; export interface OnboardContext { step: (current: number, total: number, message: string) => void; diff --git a/src/lib/config-io.ts b/src/lib/config-io.ts index 5e28967ca7..fa7f3e3e4a 100644 --- a/src/lib/config-io.ts +++ b/src/lib/config-io.ts @@ -8,8 +8,10 @@ import os from "node:os"; import path from "node:path"; import { shellQuote } from "./shell-quote"; +import { isErrnoException, isPermissionError } from "./errno"; -type ErrnoLike = Error | { code?: string | number } | null; +// Strict JSON types for file serialization — unlike json-types.ts, +// these exclude undefined since actual JSON cannot contain it. type JsonScalar = string | number | boolean | null; type JsonValue = JsonScalar | JsonObject | JsonValue[]; type JsonObject = { [key: string]: JsonValue }; @@ -19,14 +21,6 @@ function toError(error: Error | string | number | boolean | null | undefined): E return error instanceof Error ? error : new Error(String(error)); } -function isErrnoException(error: ErrnoLike): error is NodeJS.ErrnoException { - return error instanceof Error && "code" in error; -} - -function isPermissionError(error: ErrnoLike): error is NodeJS.ErrnoException { - return isErrnoException(error) && (error.code === "EACCES" || error.code === "EPERM"); -} - function parseJson(text: string): T { return JSON.parse(text); } diff --git a/src/lib/credentials.ts b/src/lib/credentials.ts index 4025074502..2b2da02b69 100644 --- a/src/lib/credentials.ts +++ b/src/lib/credentials.ts @@ -8,15 +8,10 @@ import path from "node:path"; import readline from "node:readline"; import { readConfigFile, writeConfigFile } from "./config-io"; +import { isErrnoException } from "./errno"; const UNSAFE_HOME_PATHS = new Set(["/tmp", "/var/tmp", "/dev/shm", "/"]); -type ErrnoLike = Error | { code?: string | number } | null; - -function isErrnoException(error: ErrnoLike): error is NodeJS.ErrnoException { - return error !== null && typeof error === "object" && "code" in error; -} - type CredentialInput = string | null | undefined; export function resolveHomeDir(): string { @@ -40,7 +35,7 @@ export function resolveHomeDir(): string { } } catch (error) { if ( - !(typeof error === "object" && error !== null && isErrnoException(error)) || + !isErrnoException(error) || error.code !== "ENOENT" ) { throw error; diff --git a/src/lib/errno.ts b/src/lib/errno.ts new file mode 100644 index 0000000000..0027a2351e --- /dev/null +++ b/src/lib/errno.ts @@ -0,0 +1,44 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +/** + * Shared errno helpers for safely narrowing caught errors to + * `NodeJS.ErrnoException`. + * + * Multiple modules (config-io, credentials, http-probe, onboard, + * onboard-session, registry) previously defined their own + * `isErrnoException` + `ErrnoLike` type. This module unifies them + * into a single source of truth. + * + * Usage in catch blocks: + * + * ```ts + * try { … } catch (error) { + * if (isErrnoException(error) && error.code === "ENOENT") { … } + * } + * ``` + */ + +/** + * Narrow an unknown caught value to `NodeJS.ErrnoException`. + * + * Accepts `unknown` so callers never need a pre-cast. Returns `true` + * when the value is a non-null object carrying a `code` or `errno` + * property — the two fields Node.js sets on filesystem / child-process + * errors. + */ +export function isErrnoException(error: unknown): error is NodeJS.ErrnoException { + return ( + typeof error === "object" && + error !== null && + ("code" in error || "errno" in error) + ); +} + +/** + * Convenience: true when the error is an `EACCES` or `EPERM` errno, + * commonly used for permission-denied guards. + */ +export function isPermissionError(error: unknown): error is NodeJS.ErrnoException { + return isErrnoException(error) && (error.code === "EACCES" || error.code === "EPERM"); +} diff --git a/src/lib/http-probe.ts b/src/lib/http-probe.ts index b93520db1f..731bf4ab42 100644 --- a/src/lib/http-probe.ts +++ b/src/lib/http-probe.ts @@ -14,13 +14,9 @@ import type { ProbeResult } from "./onboard-types"; import { ROOT } from "./paths"; import { compactText } from "./url-utils"; -export type CurlProbeResult = ProbeResult; - -type ErrnoLike = Error | { code?: string | number; errno?: string | number } | null; +import { isErrnoException } from "./errno"; -function isErrnoException(error: ErrnoLike): error is NodeJS.ErrnoException { - return error !== null && typeof error === "object" && ("code" in error || "errno" in error); -} +export type CurlProbeResult = ProbeResult; export interface CurlProbeOptions { cwd?: string; diff --git a/src/lib/json-types.ts b/src/lib/json-types.ts new file mode 100644 index 0000000000..cb65ac8480 --- /dev/null +++ b/src/lib/json-types.ts @@ -0,0 +1,27 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +/** + * Shared recursive JSON-like types for loosely-typed data boundaries. + * + * Several modules (onboard, agent-onboard, policies, onboard-session) + * defined their own Scalar / Value / Object triples for the same + * purpose: representing data parsed from JSON, YAML, or environment + * variables before it is validated into a narrower domain type. + * + * This module provides a single canonical set so the pattern is + * consistent across the CLI codebase. + * + * Note: The plugin (`nemoclaw/src/`) has its own parallel types + * (`PluginScalar`, `PluginValue`, `PluginRecord`) because the plugin + * and CLI are compiled separately and cannot share imports. + */ + +/** A single JSON-compatible scalar (includes `undefined` for optional fields). */ +export type JsonScalar = string | number | boolean | null | undefined; + +/** A recursive JSON-compatible value: scalar, object, or array. */ +export type JsonValue = JsonScalar | JsonObject | JsonValue[]; + +/** A JSON-compatible object with string keys and recursive values. */ +export type JsonObject = { [key: string]: JsonValue }; diff --git a/src/lib/onboard-session.ts b/src/lib/onboard-session.ts index 1acb0427e9..cd6bdc948f 100644 --- a/src/lib/onboard-session.ts +++ b/src/lib/onboard-session.ts @@ -11,6 +11,7 @@ import fs from "node:fs"; import path from "node:path"; import { redactSensitiveText, redactUrl } from "./redact"; +import { isErrnoException } from "./errno"; import type { WebSearchConfig } from "./web-search"; export const SESSION_VERSION = 1; @@ -18,9 +19,11 @@ export const SESSION_DIR = path.join(process.env.HOME || "/tmp", ".nemoclaw"); export const SESSION_FILE = path.join(SESSION_DIR, "onboard-session.json"); export const LOCK_FILE = path.join(SESSION_DIR, "onboard.lock"); -type SessionJsonPrimitive = string | number | boolean | null; -type SessionJsonValue = SessionJsonPrimitive | UnknownRecord | SessionJsonValue[]; -type UnknownRecord = { [key: string]: SessionJsonValue }; +import type { JsonValue, JsonObject } from "./json-types"; + +// Session-specific aliases for the shared JSON types. +type SessionJsonValue = JsonValue; +type UnknownRecord = JsonObject; type StepStatus = "pending" | "in_progress" | "complete" | "failed" | "skipped"; const STEP_STATES: readonly StepStatus[] = [ @@ -160,12 +163,6 @@ export function isObject(value: unknown): value is UnknownRecord { return typeof value === "object" && value !== null && !Array.isArray(value); } -type ErrnoLike = Error | { code?: string | number } | null; - -function isErrnoException(error: ErrnoLike): error is NodeJS.ErrnoException { - return error !== null && error instanceof Error && "code" in error; -} - function readString(value: SessionJsonValue | undefined): string | null { return typeof value === "string" ? value : null; } @@ -366,7 +363,7 @@ function isProcessAlive(pid: number): boolean { process.kill(pid, 0); return true; } catch (error) { - return error instanceof Error && isErrnoException(error) && error.code === "EPERM"; + return isErrnoException(error) && error.code === "EPERM"; } } @@ -406,7 +403,7 @@ export function acquireOnboardLock(command: string | null = null): LockResult { // resolves to the same file we created (fstat ino vs stat ino). fd = fs.openSync(LOCK_FILE, "wx", 0o600); } catch (error) { - if (!(error instanceof Error && isErrnoException(error)) || error.code !== "EEXIST") { + if (!isErrnoException(error) || error.code !== "EEXIST") { throw error; } @@ -423,11 +420,7 @@ export function acquireOnboardLock(command: string | null = null): LockResult { staleInode = stat.ino; existing = parseLockFile(fs.readFileSync(LOCK_FILE, "utf8")); } catch (readError) { - if ( - readError instanceof Error && - isErrnoException(readError) && - readError.code === "ENOENT" - ) { + if (isErrnoException(readError) && readError.code === "ENOENT") { continue; } throw readError; @@ -505,7 +498,7 @@ function unlinkIfInodeMatches(filePath: string, expectedInode: bigint | null): v return; } } catch (statError) { - if (statError instanceof Error && isErrnoException(statError) && statError.code === "ENOENT") { + if (isErrnoException(statError) && statError.code === "ENOENT") { return; } throw statError; @@ -513,13 +506,7 @@ function unlinkIfInodeMatches(filePath: string, expectedInode: bigint | null): v try { fs.unlinkSync(filePath); } catch (unlinkError) { - if ( - !( - unlinkError instanceof Error && - isErrnoException(unlinkError) && - unlinkError.code === "ENOENT" - ) - ) { + if (!isErrnoException(unlinkError) || unlinkError.code !== "ENOENT") { throw unlinkError; } } @@ -540,7 +527,7 @@ export function releaseOnboardLock(): void { const pathStat = fs.statSync(LOCK_FILE, { bigint: true }); pathInode = pathStat.ino; } catch (error) { - if (!(error instanceof Error && isErrnoException(error) && error.code === "ENOENT")) { + if (!(isErrnoException(error) && error.code === "ENOENT")) { // Unexpected — fall through to closing the fd. } } @@ -548,13 +535,7 @@ export function releaseOnboardLock(): void { try { fs.unlinkSync(LOCK_FILE); } catch (unlinkError) { - if ( - !( - unlinkError instanceof Error && - isErrnoException(unlinkError) && - unlinkError.code === "ENOENT" - ) - ) { + if (!(isErrnoException(unlinkError) && unlinkError.code === "ENOENT")) { // Best effort — surfacing this would mask the real error. } } @@ -582,7 +563,7 @@ export function releaseOnboardLock(): void { try { existing = parseLockFile(fs.readFileSync(LOCK_FILE, "utf8")); } catch (error) { - if (error instanceof Error && isErrnoException(error) && error.code === "ENOENT") return; + if (isErrnoException(error) && error.code === "ENOENT") return; throw error; } if (!existing) return; diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index 056c8c10d1..129ab524b1 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -27,6 +27,8 @@ const LOCAL_INFERENCE_TIMEOUT_SECS = envInt("NEMOCLAW_LOCAL_INFERENCE_TIMEOUT", const ANSI_RE = /\x1B(?:\[[0-?]*[ -/]*[@-~]|\][^\x07]*(?:\x07|\x1B\\)|[@-_])/g; const runner: typeof import("./runner") = require("./runner"); const { ROOT, SCRIPTS, redact, run, runCapture, runFile, shellQuote, validateName } = runner; +const errnoUtils: typeof import("./errno") = require("./errno"); +const { isErrnoException } = errnoUtils; type RunnerOptions = { env?: NodeJS.ProcessEnv; @@ -37,10 +39,6 @@ type RunnerOptions = { openshellBinary?: string; }; -function isErrnoException(error: object | null): error is NodeJS.ErrnoException { - return error !== null && "code" in error; -} - function parseJson(text: string): T { return JSON.parse(text); } @@ -237,9 +235,9 @@ type RemoteProviderConfigEntry = { skipVerify?: boolean; }; -type LooseScalar = string | number | boolean | null | undefined; -type LooseValue = LooseScalar | LooseObject | LooseValue[]; -type LooseObject = { [key: string]: LooseValue }; +// Re-export shared JSON types under the names used throughout this module. +// See src/lib/json-types.ts for the canonical definitions. +import type { JsonScalar as LooseScalar, JsonValue as LooseValue, JsonObject as LooseObject } from "./json-types"; type OnboardOptions = { nonInteractive?: boolean; diff --git a/src/lib/policies.ts b/src/lib/policies.ts index 08e0e2da74..055b5b80c5 100644 --- a/src/lib/policies.ts +++ b/src/lib/policies.ts @@ -3,6 +3,8 @@ // // Policy preset management — list, load, merge, and apply presets. +import type { JsonValue, JsonObject } from "./json-types"; + const fs = require("fs"); const path = require("path"); const os = require("os"); @@ -20,9 +22,9 @@ type PresetInfo = { description: string; }; -type PolicyScalar = string | number | boolean | null | undefined; -type PolicyValue = PolicyScalar | PolicyObject | PolicyValue[]; -type PolicyObject = { [key: string]: PolicyValue }; +// Re-use shared JSON types under policy-domain names. +type PolicyValue = JsonValue; +type PolicyObject = JsonObject; type PolicyDocument = PolicyObject & { version?: number; diff --git a/src/lib/registry.ts b/src/lib/registry.ts index 7975dd3005..edb0ba0550 100644 --- a/src/lib/registry.ts +++ b/src/lib/registry.ts @@ -5,12 +5,7 @@ import fs from "node:fs"; import path from "node:path"; import { ensureConfigDir, readConfigFile, writeConfigFile } from "./config-io"; - -type ErrnoLike = Error | { code?: string | number } | null; - -function isErrnoException(error: ErrnoLike): error is NodeJS.ErrnoException { - return error !== null && typeof error === "object" && "code" in error; -} +import { isErrnoException } from "./errno"; export interface SandboxEntry { name: string; @@ -74,7 +69,7 @@ export function acquireLock(): void { return; } catch (error) { if ( - !(typeof error === "object" && error !== null && isErrnoException(error)) || + !isErrnoException(error) || error.code !== "EEXIST" ) { throw error; @@ -90,7 +85,7 @@ export function acquireLock(): void { alive = true; } catch (killErr) { alive = - typeof killErr === "object" && killErr !== null && isErrnoException(killErr) + isErrnoException(killErr) ? killErr.code === "EPERM" : false; } @@ -127,7 +122,7 @@ export function releaseLock(): void { fs.unlinkSync(LOCK_OWNER); } catch (error) { if ( - !(typeof error === "object" && error !== null && isErrnoException(error)) || + !isErrnoException(error) || error.code !== "ENOENT" ) { throw error; @@ -137,7 +132,7 @@ export function releaseLock(): void { fs.rmSync(LOCK_DIR, { recursive: true, force: true }); } catch (error) { if ( - !(typeof error === "object" && error !== null && isErrnoException(error)) || + !isErrnoException(error) || error.code !== "ENOENT" ) { throw error; diff --git a/src/lib/usage-notice.ts b/src/lib/usage-notice.ts index fe04600110..1fe31800a4 100644 --- a/src/lib/usage-notice.ts +++ b/src/lib/usage-notice.ts @@ -59,6 +59,10 @@ function parseJson(text: string): T { return JSON.parse(text); } +// Reflect.get is used throughout the codebase as a type-safe alternative to +// direct property access on loosely-typed objects. Unlike an `as Record<…>` +// cast it never widens the target type and avoids eslint no-unsafe-member-access +// warnings. See also: deploy.ts, onboard.ts, ws-proxy-fix.ts. function readStringProperty(value: object | null, key: string): string | undefined { if (!value) { return undefined; diff --git a/test/errno.test.ts b/test/errno.test.ts new file mode 100644 index 0000000000..0a12498559 --- /dev/null +++ b/test/errno.test.ts @@ -0,0 +1,82 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { describe, it, expect } from "vitest"; +import { isErrnoException, isPermissionError } from "../src/lib/errno"; + +describe("isErrnoException", () => { + it("returns true for Error with code property", () => { + const err = Object.assign(new Error("ENOENT"), { code: "ENOENT" }); + expect(isErrnoException(err)).toBe(true); + }); + + it("returns true for plain object with code property", () => { + expect(isErrnoException({ code: "EACCES" })).toBe(true); + }); + + it("returns true for object with errno property", () => { + expect(isErrnoException({ errno: -2 })).toBe(true); + }); + + it("returns true for Error with both code and errno", () => { + const err = Object.assign(new Error("fail"), { code: "ENOENT", errno: -2 }); + expect(isErrnoException(err)).toBe(true); + }); + + it("returns false for null", () => { + expect(isErrnoException(null)).toBe(false); + }); + + it("returns false for undefined", () => { + expect(isErrnoException(undefined)).toBe(false); + }); + + it("returns false for a string", () => { + expect(isErrnoException("ENOENT")).toBe(false); + }); + + it("returns false for a number", () => { + expect(isErrnoException(42)).toBe(false); + }); + + it("returns false for a plain Error without code/errno", () => { + expect(isErrnoException(new Error("oops"))).toBe(false); + }); + + it("returns false for an empty object", () => { + expect(isErrnoException({})).toBe(false); + }); + + it("narrows the type so .code is accessible", () => { + const err: unknown = Object.assign(new Error("fail"), { code: "EPERM" }); + if (isErrnoException(err)) { + // TypeScript should allow this without a cast. + expect(err.code).toBe("EPERM"); + } else { + throw new Error("Expected isErrnoException to return true"); + } + }); +}); + +describe("isPermissionError", () => { + it("returns true for EACCES", () => { + const err = Object.assign(new Error("permission denied"), { code: "EACCES" }); + expect(isPermissionError(err)).toBe(true); + }); + + it("returns true for EPERM", () => { + const err = Object.assign(new Error("not permitted"), { code: "EPERM" }); + expect(isPermissionError(err)).toBe(true); + }); + + it("returns false for ENOENT", () => { + const err = Object.assign(new Error("not found"), { code: "ENOENT" }); + expect(isPermissionError(err)).toBe(false); + }); + + it("returns false for non-errno values", () => { + expect(isPermissionError(null)).toBe(false); + expect(isPermissionError("EACCES")).toBe(false); + expect(isPermissionError(new Error("oops"))).toBe(false); + }); +});