Config: fail closed invalid config loads (#39071)

* Config: fail closed invalid config loads

* CLI: keep diagnostics on explicit best-effort config

* Tests: cover invalid config best-effort diagnostics

* Changelog: note invalid config fail-closed fix

* Status: pass best-effort config through status-all gateway RPCs

* CLI: pass config through gateway secret RPC

* CLI: skip plugin loading from invalid config

* Tests: align daemon token drift env precedence
This commit is contained in:
Vincent Koc
2026-03-07 20:48:13 -05:00
committed by GitHub
parent 1831dbb63f
commit 2c7fb54956
27 changed files with 178 additions and 71 deletions

View File

@@ -132,6 +132,7 @@ describe("resolveCommandSecretRefsViaGateway", () => {
});
expect(callGateway).toHaveBeenCalledWith(
expect.objectContaining({
config,
method: "secrets.resolve",
requiredMethods: ["secrets.resolve"],
params: {

View File

@@ -396,6 +396,7 @@ export async function resolveCommandSecretRefsViaGateway(params: {
let payload: GatewaySecretsResolveResult;
try {
payload = await callGateway<GatewaySecretsResolveResult>({
config: params.config,
method: "secrets.resolve",
requiredMethods: ["secrets.resolve"],
params: {

View File

@@ -20,4 +20,27 @@ describe("resolveGatewayTokenForDriftCheck", () => {
expect(token).toBe("config-token");
});
it("does not fall back to caller env for unresolved config token refs", () => {
expect(() =>
resolveGatewayTokenForDriftCheck({
cfg: {
secrets: {
providers: {
default: { source: "env" },
},
},
gateway: {
mode: "local",
auth: {
token: { source: "env", provider: "default", id: "OPENCLAW_GATEWAY_TOKEN" },
},
},
} as OpenClawConfig,
env: {
OPENCLAW_GATEWAY_TOKEN: "env-token",
} as NodeJS.ProcessEnv,
}),
).toThrow(/gateway\.auth\.token/i);
});
});

View File

@@ -7,10 +7,10 @@ export function resolveGatewayTokenForDriftCheck(params: {
}) {
return resolveGatewayCredentialsFromConfig({
cfg: params.cfg,
env: params.env,
env: {} as NodeJS.ProcessEnv,
modeOverride: "local",
// Drift checks should compare the persisted gateway token against the
// service token, not let an exported shell env mask config drift.
// Drift checks should compare the configured local token source against the
// persisted service token, not let exported shell env hide stale service state.
localTokenPrecedence: "config-first",
}).token;
}

View File

@@ -52,6 +52,7 @@ const service = vi.hoisted(() => ({
vi.mock("../../config/config.js", () => ({
loadConfig: loadConfigMock,
readBestEffortConfig: loadConfigMock,
readConfigFileSnapshot: readConfigFileSnapshotMock,
resolveGatewayPort: resolveGatewayPortMock,
writeConfigFile: writeConfigFileMock,

View File

@@ -4,7 +4,7 @@ import {
isGatewayDaemonRuntime,
} from "../../commands/daemon-runtime.js";
import { resolveGatewayInstallToken } from "../../commands/gateway-install-token.js";
import { loadConfig, resolveGatewayPort } from "../../config/config.js";
import { readBestEffortConfig, resolveGatewayPort } from "../../config/config.js";
import { resolveIsNixMode } from "../../config/paths.js";
import { resolveGatewayService } from "../../daemon/service.js";
import { isNonFatalSystemdInstallProbeError } from "../../daemon/systemd.js";
@@ -27,7 +27,7 @@ export async function runDaemonInstall(opts: DaemonInstallOptions) {
return;
}
const cfg = loadConfig();
const cfg = await readBestEffortConfig();
const portOverride = parsePort(opts.port);
if (opts.port !== undefined && portOverride === null) {
fail("Invalid port");

View File

@@ -32,6 +32,7 @@ const service = {
vi.mock("../../config/config.js", () => ({
loadConfig: () => loadConfig(),
readBestEffortConfig: async () => loadConfig(),
}));
vi.mock("../../runtime.js", () => ({
@@ -87,7 +88,7 @@ describe("runServiceRestart token drift", () => {
);
});
it("uses gateway.auth.token when checking drift", async () => {
it("compares restart drift against config token even when caller env is set", async () => {
loadConfig.mockReturnValue({
gateway: {
auth: {

View File

@@ -1,11 +1,13 @@
import type { Writable } from "node:stream";
import { loadConfig } from "../../config/config.js";
import { readBestEffortConfig } from "../../config/config.js";
import { resolveIsNixMode } from "../../config/paths.js";
import { checkTokenDrift } from "../../daemon/service-audit.js";
import type { GatewayService } from "../../daemon/service.js";
import { renderSystemdUnavailableHints } from "../../daemon/systemd-hints.js";
import { isSystemdUserServiceAvailable } from "../../daemon/systemd.js";
import { isGatewaySecretRefUnavailableError } from "../../gateway/credentials.js";
import {
isGatewaySecretRefUnavailableError,
} from "../../gateway/credentials.js";
import { isWSL } from "../../infra/wsl.js";
import { defaultRuntime } from "../../runtime.js";
import { resolveGatewayTokenForDriftCheck } from "./gateway-token-drift.js";
@@ -281,7 +283,7 @@ export async function runServiceRestart(params: {
try {
const command = await params.service.readCommand(process.env);
const serviceToken = command?.environment?.OPENCLAW_GATEWAY_TOKEN;
const cfg = loadConfig();
const cfg = await readBestEffortConfig();
const configToken = resolveGatewayTokenForDriftCheck({ cfg, env: process.env });
const driftIssue = checkTokenDrift({ serviceToken, configToken });
if (driftIssue) {

View File

@@ -33,6 +33,7 @@ const loadConfig = vi.fn(() => ({}));
vi.mock("../../config/config.js", () => ({
loadConfig: () => loadConfig(),
readBestEffortConfig: async () => loadConfig(),
resolveGatewayPort,
}));

View File

@@ -1,4 +1,4 @@
import { loadConfig, resolveGatewayPort } from "../../config/config.js";
import { readBestEffortConfig, resolveGatewayPort } from "../../config/config.js";
import { resolveGatewayService } from "../../daemon/service.js";
import { defaultRuntime } from "../../runtime.js";
import { theme } from "../../terminal/theme.js";
@@ -32,7 +32,7 @@ async function resolveGatewayRestartPort() {
} as NodeJS.ProcessEnv;
const portFromArgs = parsePortFromArgs(command?.programArguments);
return portFromArgs ?? resolveGatewayPort(loadConfig(), mergedEnv);
return portFromArgs ?? resolveGatewayPort(await readBestEffortConfig(), mergedEnv);
}
export async function runDaemonUninstall(opts: DaemonLifecycleOptions = {}) {
@@ -70,8 +70,8 @@ export async function runDaemonStop(opts: DaemonLifecycleOptions = {}) {
export async function runDaemonRestart(opts: DaemonLifecycleOptions = {}): Promise<boolean> {
const json = Boolean(opts.json);
const service = resolveGatewayService();
const restartPort = await resolveGatewayRestartPort().catch(() =>
resolveGatewayPort(loadConfig(), process.env),
const restartPort = await resolveGatewayRestartPort().catch(async () =>
resolveGatewayPort(await readBestEffortConfig(), process.env),
);
const restartWaitMs = POST_RESTART_HEALTH_ATTEMPTS * POST_RESTART_HEALTH_DELAY_MS;
const restartWaitSeconds = Math.round(restartWaitMs / 1000);

View File

@@ -1,9 +1,11 @@
import type { Command } from "commander";
import type { OpenClawConfig } from "../../config/config.js";
import { callGateway } from "../../gateway/call.js";
import { GATEWAY_CLIENT_MODES, GATEWAY_CLIENT_NAMES } from "../../utils/message-channel.js";
import { withProgress } from "../progress.js";
export type GatewayRpcOpts = {
config?: OpenClawConfig;
url?: string;
token?: string;
password?: string;
@@ -30,6 +32,7 @@ export const callGatewayCli = async (method: string, opts: GatewayRpcOpts, param
},
async () =>
await callGateway({
config: opts.config,
url: opts.url,
token: opts.token,
password: opts.password,

View File

@@ -1,7 +1,7 @@
import type { Command } from "commander";
import { gatewayStatusCommand } from "../../commands/gateway-status.js";
import { formatHealthChannelLines, type HealthSummary } from "../../commands/health.js";
import { loadConfig } from "../../config/config.js";
import { readBestEffortConfig } from "../../config/config.js";
import { discoverGatewayBeacons } from "../../infra/bonjour-discovery.js";
import type { CostUsageSummary } from "../../infra/session-cost-usage.js";
import { resolveWideAreaDiscoveryDomain } from "../../infra/widearea-dns.js";
@@ -120,8 +120,9 @@ export function registerGatewayCli(program: Command) {
.action(async (method, opts, command) => {
await runGatewayCommand(async () => {
const rpcOpts = resolveGatewayRpcOptions(opts, command);
const config = await readBestEffortConfig();
const params = JSON.parse(String(opts.params ?? "{}"));
const result = await callGatewayCli(method, rpcOpts, params);
const result = await callGatewayCli(method, { ...rpcOpts, config }, params);
if (rpcOpts.json) {
defaultRuntime.log(JSON.stringify(result, null, 2));
return;
@@ -144,7 +145,8 @@ export function registerGatewayCli(program: Command) {
await runGatewayCommand(async () => {
const rpcOpts = resolveGatewayRpcOptions(opts, command);
const days = parseDaysOption(opts.days);
const result = await callGatewayCli("usage.cost", rpcOpts, { days });
const config = await readBestEffortConfig();
const result = await callGatewayCli("usage.cost", { ...rpcOpts, config }, { days });
if (rpcOpts.json) {
defaultRuntime.log(JSON.stringify(result, null, 2));
return;
@@ -165,7 +167,8 @@ export function registerGatewayCli(program: Command) {
.action(async (opts, command) => {
await runGatewayCommand(async () => {
const rpcOpts = resolveGatewayRpcOptions(opts, command);
const result = await callGatewayCli("health", rpcOpts);
const config = await readBestEffortConfig();
const result = await callGatewayCli("health", { ...rpcOpts, config });
if (rpcOpts.json) {
defaultRuntime.log(JSON.stringify(result, null, 2));
return;
@@ -211,7 +214,7 @@ export function registerGatewayCli(program: Command) {
.option("--json", "Output JSON", false)
.action(async (opts: GatewayDiscoverOpts) => {
await runGatewayCommand(async () => {
const cfg = loadConfig();
const cfg = await readBestEffortConfig();
const wideAreaDomain = resolveWideAreaDiscoveryDomain({
configDomain: cfg.discovery?.wideArea?.domain,
});

View File

@@ -18,10 +18,17 @@ const { nodesAction, registerNodesCli } = vi.hoisted(() => {
return { nodesAction: action, registerNodesCli: register };
});
const configModule = vi.hoisted(() => ({
loadConfig: vi.fn(),
readConfigFileSnapshot: vi.fn(),
}));
vi.mock("../acp-cli.js", () => ({ registerAcpCli }));
vi.mock("../nodes-cli.js", () => ({ registerNodesCli }));
vi.mock("../../config/config.js", () => configModule);
const { registerSubCliByName, registerSubCliCommands } = await import("./register.subclis.js");
const { loadValidatedConfigForPluginRegistration, registerSubCliByName, registerSubCliCommands } =
await import("./register.subclis.js");
describe("registerSubCliCommands", () => {
const originalArgv = process.argv;
@@ -47,6 +54,8 @@ describe("registerSubCliCommands", () => {
acpAction.mockClear();
registerNodesCli.mockClear();
nodesAction.mockClear();
configModule.loadConfig.mockReset();
configModule.readConfigFileSnapshot.mockReset();
});
afterEach(() => {
@@ -79,6 +88,28 @@ describe("registerSubCliCommands", () => {
expect(registerAcpCli).not.toHaveBeenCalled();
});
it("returns null for plugin registration when the config snapshot is invalid", async () => {
configModule.readConfigFileSnapshot.mockResolvedValueOnce({
valid: false,
config: { plugins: { load: { paths: ["/tmp/evil"] } } },
});
await expect(loadValidatedConfigForPluginRegistration()).resolves.toBeNull();
expect(configModule.loadConfig).not.toHaveBeenCalled();
});
it("loads validated config for plugin registration when the snapshot is valid", async () => {
const loadedConfig = { plugins: { enabled: true } };
configModule.readConfigFileSnapshot.mockResolvedValueOnce({
valid: true,
config: loadedConfig,
});
configModule.loadConfig.mockReturnValueOnce(loadedConfig);
await expect(loadValidatedConfigForPluginRegistration()).resolves.toBe(loadedConfig);
expect(configModule.loadConfig).toHaveBeenCalledTimes(1);
});
it("re-parses argv for lazy subcommands", async () => {
const program = createRegisteredProgram(["node", "openclaw", "nodes", "list"], "openclaw");

View File

@@ -28,10 +28,15 @@ const shouldEagerRegisterSubcommands = (_argv: string[]) => {
return isTruthyEnvValue(process.env.OPENCLAW_DISABLE_LAZY_SUBCOMMANDS);
};
const loadConfig = async (): Promise<OpenClawConfig> => {
const mod = await import("../../config/config.js");
return mod.loadConfig();
};
export const loadValidatedConfigForPluginRegistration =
async (): Promise<OpenClawConfig | null> => {
const mod = await import("../../config/config.js");
const snapshot = await mod.readConfigFileSnapshot();
if (!snapshot.valid) {
return null;
}
return mod.loadConfig();
};
// Note for humans and agents:
// If you update the list of commands, also check whether they have subcommands
@@ -217,7 +222,10 @@ const entries: SubCliEntry[] = [
// The pairing CLI calls listPairingChannels() at registration time,
// which requires the plugin registry to be populated with channel plugins.
const { registerPluginCliCommands } = await import("../../plugins/cli.js");
registerPluginCliCommands(program, await loadConfig());
const config = await loadValidatedConfigForPluginRegistration();
if (config) {
registerPluginCliCommands(program, config);
}
const mod = await import("../pairing-cli.js");
mod.registerPairingCli(program);
},
@@ -230,7 +238,10 @@ const entries: SubCliEntry[] = [
const mod = await import("../plugins-cli.js");
mod.registerPluginsCli(program);
const { registerPluginCliCommands } = await import("../../plugins/cli.js");
registerPluginCliCommands(program, await loadConfig());
const config = await loadValidatedConfigForPluginRegistration();
if (config) {
registerPluginCliCommands(program, config);
}
},
},
{

View File

@@ -126,8 +126,12 @@ export async function runCli(argv: string[] = process.argv) {
if (!shouldSkipPluginRegistration) {
// Register plugin CLI commands before parsing
const { registerPluginCliCommands } = await import("../plugins/cli.js");
const { loadConfig } = await import("../config/config.js");
registerPluginCliCommands(program, loadConfig());
const { loadValidatedConfigForPluginRegistration } =
await import("./program/register.subclis.js");
const config = await loadValidatedConfigForPluginRegistration();
if (config) {
registerPluginCliCommands(program, config);
}
}
await program.parseAsync(parseArgv);