Config: sanitize validation logs and keep systemd restarts
This commit is contained in:
parent
bc3565f5f4
commit
2d84590373
@ -102,4 +102,48 @@ describe("config io warning/error logging", () => {
|
|||||||
expect(logger.error).toHaveBeenCalledTimes(2);
|
expect(logger.error).toHaveBeenCalledTimes(2);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("sanitizes config validation details before logging", async () => {
|
||||||
|
vi.resetModules();
|
||||||
|
vi.doMock("./validation.js", async (importOriginal) => {
|
||||||
|
const actual = await importOriginal<typeof import("./validation.js")>();
|
||||||
|
return {
|
||||||
|
...actual,
|
||||||
|
validateConfigObjectWithPlugins: vi.fn(() => ({
|
||||||
|
ok: false as const,
|
||||||
|
issues: [
|
||||||
|
{
|
||||||
|
path: "plugins.entries.bad\nkey\u001b[31m",
|
||||||
|
message: "invalid\tvalue\r\n\u001b[2J",
|
||||||
|
},
|
||||||
|
],
|
||||||
|
})),
|
||||||
|
};
|
||||||
|
});
|
||||||
|
|
||||||
|
const { createConfigIO: createConfigIOWithMock } = await import("./io.js");
|
||||||
|
const home = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-config-log-sanitize-"));
|
||||||
|
const configDir = path.join(home, ".openclaw");
|
||||||
|
const configPath = path.join(configDir, "openclaw.json");
|
||||||
|
await fs.mkdir(configDir, { recursive: true });
|
||||||
|
await fs.writeFile(configPath, JSON.stringify({ gateway: { port: 18789 } }, null, 2));
|
||||||
|
const logger = { warn: vi.fn(), error: vi.fn() };
|
||||||
|
const io = createConfigIOWithMock({
|
||||||
|
env: {} as NodeJS.ProcessEnv,
|
||||||
|
homedir: () => home,
|
||||||
|
logger,
|
||||||
|
});
|
||||||
|
|
||||||
|
try {
|
||||||
|
expect(io.loadConfig()).toEqual({});
|
||||||
|
const logged = logger.error.mock.calls[0]?.[0] ?? "";
|
||||||
|
expect(logged).toContain("plugins.entries.bad\\nkey");
|
||||||
|
expect(logged).toContain("invalid\\tvalue\\r\\n");
|
||||||
|
expect(logged).not.toContain("\u001b");
|
||||||
|
} finally {
|
||||||
|
vi.doUnmock("./validation.js");
|
||||||
|
vi.resetModules();
|
||||||
|
await fs.rm(home, { recursive: true, force: true });
|
||||||
|
}
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@ -13,6 +13,7 @@ import {
|
|||||||
shouldDeferShellEnvFallback,
|
shouldDeferShellEnvFallback,
|
||||||
shouldEnableShellEnvFallback,
|
shouldEnableShellEnvFallback,
|
||||||
} from "../infra/shell-env.js";
|
} from "../infra/shell-env.js";
|
||||||
|
import { sanitizeTerminalText } from "../terminal/safe-text.js";
|
||||||
import { VERSION } from "../version.js";
|
import { VERSION } from "../version.js";
|
||||||
import { DuplicateAgentDirError, findDuplicateAgentDirs } from "./agent-dirs.js";
|
import { DuplicateAgentDirError, findDuplicateAgentDirs } from "./agent-dirs.js";
|
||||||
import { maintainConfigBackups } from "./backup-rotation.js";
|
import { maintainConfigBackups } from "./backup-rotation.js";
|
||||||
@ -591,6 +592,16 @@ function clearConfigMessageFingerprint(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function formatConfigIssueDetails(issues: Array<{ path: string; message: string }>): string {
|
||||||
|
return issues
|
||||||
|
.map((iss) => {
|
||||||
|
const safePath = sanitizeTerminalText(iss.path || "<root>");
|
||||||
|
const safeMessage = sanitizeTerminalText(iss.message);
|
||||||
|
return `- ${safePath}: ${safeMessage}`;
|
||||||
|
})
|
||||||
|
.join("\n");
|
||||||
|
}
|
||||||
|
|
||||||
function getConfigMiskeyWarnings(raw: unknown): string[] {
|
function getConfigMiskeyWarnings(raw: unknown): string[] {
|
||||||
const warnings: string[] = [];
|
const warnings: string[] = [];
|
||||||
if (!raw || typeof raw !== "object") {
|
if (!raw || typeof raw !== "object") {
|
||||||
@ -765,26 +776,24 @@ export function createConfigIO(overrides: ConfigIoDeps = {}) {
|
|||||||
}
|
}
|
||||||
const validated = validateConfigObjectWithPlugins(resolvedConfig);
|
const validated = validateConfigObjectWithPlugins(resolvedConfig);
|
||||||
if (!validated.ok) {
|
if (!validated.ok) {
|
||||||
const details = validated.issues
|
const details = formatConfigIssueDetails(validated.issues);
|
||||||
.map((iss) => `- ${iss.path || "<root>"}: ${iss.message}`)
|
|
||||||
.join("\n");
|
|
||||||
clearConfigMessageFingerprint(configPath, "warnings");
|
clearConfigMessageFingerprint(configPath, "warnings");
|
||||||
logConfigMessageOnce({
|
logConfigMessageOnce({
|
||||||
configPath,
|
configPath,
|
||||||
kind: "invalid",
|
kind: "invalid",
|
||||||
message: `Invalid config at ${configPath}:\\n${details}`,
|
message: `Invalid config at ${sanitizeTerminalText(configPath)}:\\n${details}`,
|
||||||
logger: deps.logger,
|
logger: deps.logger,
|
||||||
});
|
});
|
||||||
const error = new Error(`Invalid config at ${configPath}:\n${details}`);
|
const error = new Error(
|
||||||
|
`Invalid config at ${sanitizeTerminalText(configPath)}:\n${details}`,
|
||||||
|
);
|
||||||
(error as { code?: string; details?: string }).code = "INVALID_CONFIG";
|
(error as { code?: string; details?: string }).code = "INVALID_CONFIG";
|
||||||
(error as { code?: string; details?: string }).details = details;
|
(error as { code?: string; details?: string }).details = details;
|
||||||
throw error;
|
throw error;
|
||||||
}
|
}
|
||||||
clearConfigMessageFingerprint(configPath, "invalid");
|
clearConfigMessageFingerprint(configPath, "invalid");
|
||||||
if (validated.warnings.length > 0) {
|
if (validated.warnings.length > 0) {
|
||||||
const details = validated.warnings
|
const details = formatConfigIssueDetails(validated.warnings);
|
||||||
.map((iss) => `- ${iss.path || "<root>"}: ${iss.message}`)
|
|
||||||
.join("\n");
|
|
||||||
warningMessages.push(`Config warnings:\\n${details}`);
|
warningMessages.push(`Config warnings:\\n${details}`);
|
||||||
}
|
}
|
||||||
const futureVersionWarning = getFutureVersionWarning(validated.config);
|
const futureVersionWarning = getFutureVersionWarning(validated.config);
|
||||||
|
|||||||
@ -21,14 +21,14 @@ describe("buildSystemdUnit", () => {
|
|||||||
expect(unit).toContain("KillMode=control-group");
|
expect(unit).toContain("KillMode=control-group");
|
||||||
});
|
});
|
||||||
|
|
||||||
it("restarts only on failure", () => {
|
it("keeps restart always for supervised restarts", () => {
|
||||||
const unit = buildSystemdUnit({
|
const unit = buildSystemdUnit({
|
||||||
description: "OpenClaw Gateway",
|
description: "OpenClaw Gateway",
|
||||||
programArguments: ["/usr/bin/openclaw", "gateway", "run"],
|
programArguments: ["/usr/bin/openclaw", "gateway", "run"],
|
||||||
environment: {},
|
environment: {},
|
||||||
});
|
});
|
||||||
expect(unit).toContain("Restart=on-failure");
|
expect(unit).toContain("Restart=always");
|
||||||
expect(unit).not.toContain("Restart=always");
|
expect(unit).not.toContain("Restart=on-failure");
|
||||||
});
|
});
|
||||||
|
|
||||||
it("rejects environment values with line breaks", () => {
|
it("rejects environment values with line breaks", () => {
|
||||||
|
|||||||
@ -57,7 +57,9 @@ export function buildSystemdUnit({
|
|||||||
"",
|
"",
|
||||||
"[Service]",
|
"[Service]",
|
||||||
`ExecStart=${execStart}`,
|
`ExecStart=${execStart}`,
|
||||||
"Restart=on-failure",
|
"Restart=always",
|
||||||
|
// systemd already has burst protection; keep a shorter retry than launchd so
|
||||||
|
// supervised restart requests still relaunch promptly on Linux.
|
||||||
"RestartSec=5",
|
"RestartSec=5",
|
||||||
// Keep service children in the same lifecycle so restarts do not leave
|
// Keep service children in the same lifecycle so restarts do not leave
|
||||||
// orphan ACP/runtime workers behind.
|
// orphan ACP/runtime workers behind.
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user