fix(logging): correct levelToMinLevel mapping and related filter logic for tslog v4 (#44646)

* fix: correct levelToMinLevel mapping and isFileLogLevelEnabled direction for tslog v4

* test: add regression tests for logging level filter and child logger inheritance

* fix: propagate minLevel to toPinoLikeLogger sub-loggers

* fix: correct shouldLogToConsole comparison direction in subsystem.ts

* test: cover logging threshold regressions

* fix(logging): treat silent as non-emittable level

---------

Co-authored-by: Altay <altay@uinaf.dev>
This commit is contained in:
zhumengzhu
2026-04-08 07:19:20 +08:00
committed by GitHub
parent 6bd480ea1f
commit f6bf8c7202
7 changed files with 199 additions and 9 deletions

View File

@@ -101,6 +101,7 @@ Docs: https://docs.openclaw.ai
- iOS/gateway: replace string-matched connection error UI with structured gateway connection problems, preserve actionable pairing/auth failures over later generic disconnect noise, and surface reusable problem banners and details across onboarding, settings, and root status surfaces. (#62650) Thanks @ngutman.
- Git/env sanitization: block additional Git repository-plumbing env variables such as `GIT_DIR`, `GIT_WORK_TREE`, `GIT_COMMON_DIR`, `GIT_INDEX_FILE`, `GIT_OBJECT_DIRECTORY`, `GIT_ALTERNATE_OBJECT_DIRECTORIES`, and `GIT_NAMESPACE` so host-run Git commands cannot be redirected to attacker-chosen repository state through inherited or request-scoped env. (#62002) Thanks @eleqtrizit.
- Host exec/env sanitization: block additional request-scoped credential and config-path overrides such as `KUBECONFIG`, cloud credential-path env, `CARGO_HOME`, and `HELM_HOME` so host-run tools can no longer be redirected to attacker-chosen config or state. (#59119) Thanks @eleqtrizit.
- Logging: make `logging.level` and `logging.consoleLevel` honor the documented severity threshold ordering again, and keep child loggers inheriting the parent `minLevel`. (#44646) Thanks @zhumengzhu.
## 2026.4.5

View File

@@ -0,0 +1,133 @@
import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
const { readLoggingConfigMock, shouldSkipMutatingLoggingConfigReadMock } = vi.hoisted(() => ({
readLoggingConfigMock: vi.fn(() => undefined),
shouldSkipMutatingLoggingConfigReadMock: vi.fn(() => false),
}));
vi.mock("./config.js", () => ({
readLoggingConfig: readLoggingConfigMock,
shouldSkipMutatingLoggingConfigRead: shouldSkipMutatingLoggingConfigReadMock,
}));
vi.mock("./node-require.js", () => ({
resolveNodeRequireFromMeta: () => () => {
throw new Error("config fallback not used");
},
}));
let logging: typeof import("../logging.js");
beforeAll(async () => {
logging = await import("../logging.js");
});
beforeEach(() => {
delete process.env.OPENCLAW_TEST_FILE_LOG;
delete process.env.OPENCLAW_LOG_LEVEL;
readLoggingConfigMock.mockClear();
shouldSkipMutatingLoggingConfigReadMock.mockReset();
shouldSkipMutatingLoggingConfigReadMock.mockReturnValue(false);
logging.resetLogger();
logging.setLoggerOverride(null);
});
afterEach(() => {
delete process.env.OPENCLAW_TEST_FILE_LOG;
delete process.env.OPENCLAW_LOG_LEVEL;
logging.resetLogger();
logging.setLoggerOverride(null);
vi.restoreAllMocks();
});
describe("isFileLogLevelEnabled", () => {
it("returns false for all levels when configured as silent", () => {
logging.setLoggerOverride({ level: "silent" });
expect(logging.isFileLogLevelEnabled("fatal")).toBe(false);
expect(logging.isFileLogLevelEnabled("error")).toBe(false);
expect(logging.isFileLogLevelEnabled("warn")).toBe(false);
expect(logging.isFileLogLevelEnabled("info")).toBe(false);
expect(logging.isFileLogLevelEnabled("debug")).toBe(false);
expect(logging.isFileLogLevelEnabled("trace")).toBe(false);
});
it("passes only fatal when configured as fatal", () => {
logging.setLoggerOverride({ level: "fatal" });
expect(logging.isFileLogLevelEnabled("fatal")).toBe(true);
expect(logging.isFileLogLevelEnabled("error")).toBe(false);
expect(logging.isFileLogLevelEnabled("warn")).toBe(false);
expect(logging.isFileLogLevelEnabled("info")).toBe(false);
expect(logging.isFileLogLevelEnabled("debug")).toBe(false);
expect(logging.isFileLogLevelEnabled("trace")).toBe(false);
});
it("passes fatal and error when configured as error", () => {
logging.setLoggerOverride({ level: "error" });
expect(logging.isFileLogLevelEnabled("fatal")).toBe(true);
expect(logging.isFileLogLevelEnabled("error")).toBe(true);
expect(logging.isFileLogLevelEnabled("warn")).toBe(false);
expect(logging.isFileLogLevelEnabled("info")).toBe(false);
expect(logging.isFileLogLevelEnabled("debug")).toBe(false);
expect(logging.isFileLogLevelEnabled("trace")).toBe(false);
});
it("passes fatal, error, warn, info when configured as info", () => {
logging.setLoggerOverride({ level: "info" });
expect(logging.isFileLogLevelEnabled("fatal")).toBe(true);
expect(logging.isFileLogLevelEnabled("error")).toBe(true);
expect(logging.isFileLogLevelEnabled("warn")).toBe(true);
expect(logging.isFileLogLevelEnabled("info")).toBe(true);
expect(logging.isFileLogLevelEnabled("debug")).toBe(false);
expect(logging.isFileLogLevelEnabled("trace")).toBe(false);
});
it("passes all levels when configured as trace", () => {
logging.setLoggerOverride({ level: "trace" });
expect(logging.isFileLogLevelEnabled("fatal")).toBe(true);
expect(logging.isFileLogLevelEnabled("error")).toBe(true);
expect(logging.isFileLogLevelEnabled("warn")).toBe(true);
expect(logging.isFileLogLevelEnabled("info")).toBe(true);
expect(logging.isFileLogLevelEnabled("debug")).toBe(true);
expect(logging.isFileLogLevelEnabled("trace")).toBe(true);
});
it("never treats silent as an emittable file level", () => {
logging.setLoggerOverride({ level: "info" });
expect(logging.isFileLogLevelEnabled("silent")).toBe(false);
});
});
describe("getChildLogger minLevel inheritance", () => {
it("child logger inherits parent minLevel when no level is specified", () => {
logging.setLoggerOverride({ level: "warn" });
const child = logging.getChildLogger({ component: "test" });
expect(child.settings.minLevel).toBe(logging.levelToMinLevel("warn"));
});
it("child logger uses its own level when explicitly specified", () => {
logging.setLoggerOverride({ level: "warn" });
const child = logging.getChildLogger({ component: "test" }, { level: "error" });
expect(child.settings.minLevel).toBe(logging.levelToMinLevel("error"));
});
it("child logger does not default to minLevel=0 (allow-all) when no level given", () => {
logging.setLoggerOverride({ level: "fatal" });
const child = logging.getChildLogger({ component: "test" });
expect(child.settings.minLevel).not.toBe(0);
expect(child.settings.minLevel).toBe(logging.levelToMinLevel("fatal"));
});
it("pino child logger propagates the parent minLevel", () => {
logging.setLoggerOverride({ level: "error" });
const base = logging.getLogger();
const getSubLoggerSpy = vi.spyOn(base, "getSubLogger");
logging.toPinoLikeLogger(base, "info").child({ component: "test" });
expect(getSubLoggerSpy).toHaveBeenCalledWith(
expect.objectContaining({
minLevel: logging.levelToMinLevel("error"),
}),
);
});
});

View File

@@ -0,0 +1,28 @@
import { describe, expect, it } from "vitest";
import { levelToMinLevel } from "./levels.js";
describe("levelToMinLevel", () => {
it("returns tslog v4 logLevelId values in ascending order", () => {
expect(levelToMinLevel("trace")).toBe(1);
expect(levelToMinLevel("debug")).toBe(2);
expect(levelToMinLevel("info")).toBe(3);
expect(levelToMinLevel("warn")).toBe(4);
expect(levelToMinLevel("error")).toBe(5);
expect(levelToMinLevel("fatal")).toBe(6);
});
it("maps silent to Infinity to suppress all logs", () => {
expect(levelToMinLevel("silent")).toBe(Number.POSITIVE_INFINITY);
});
it("fatal has a higher value than trace (not inverted)", () => {
expect(levelToMinLevel("fatal")).toBeGreaterThan(levelToMinLevel("trace"));
});
it("each level is strictly more restrictive than the one below it", () => {
const ordered = ["trace", "debug", "info", "warn", "error", "fatal"] as const;
for (let i = 1; i < ordered.length; i++) {
expect(levelToMinLevel(ordered[i])).toBeGreaterThan(levelToMinLevel(ordered[i - 1]));
}
});
});

View File

@@ -23,14 +23,15 @@ export function normalizeLogLevel(level?: string, fallback: LogLevel = "info") {
}
export function levelToMinLevel(level: LogLevel): number {
// tslog level ordering: fatal=0, error=1, warn=2, info=3, debug=4, trace=5
// tslog v4 logLevelId (src/index.ts): silly=0, trace=1, debug=2, info=3, warn=4, error=5, fatal=6
// tslog filters: logLevelId < minLevel is dropped, so higher minLevel = more restrictive.
const map: Record<LogLevel, number> = {
fatal: 0,
error: 1,
warn: 2,
trace: 1,
debug: 2,
info: 3,
debug: 4,
trace: 5,
warn: 4,
error: 5,
fatal: 6,
silent: Number.POSITIVE_INFINITY,
};
return map[level];

View File

@@ -148,10 +148,13 @@ export function isFileLogLevelEnabled(level: LogLevel): boolean {
if (!loggingState.cachedSettings) {
loggingState.cachedSettings = settings;
}
if (level === "silent") {
return false;
}
if (settings.level === "silent") {
return false;
}
return levelToMinLevel(level) <= levelToMinLevel(settings.level);
return levelToMinLevel(level) >= levelToMinLevel(settings.level);
}
function buildLogger(settings: ResolvedSettings): TsLogger<LogObj> {
@@ -254,7 +257,7 @@ export function getChildLogger(
opts?: { level?: LogLevel },
): TsLogger<LogObj> {
const base = getLogger();
const minLevel = opts?.level ? levelToMinLevel(opts.level) : undefined;
const minLevel = opts?.level ? levelToMinLevel(opts.level) : base.settings.minLevel;
const name = bindings ? JSON.stringify(bindings) : undefined;
return base.getSubLogger({
name,
@@ -269,6 +272,7 @@ export function toPinoLikeLogger(logger: TsLogger<LogObj>, level: LogLevel): Pin
toPinoLikeLogger(
logger.getSubLogger({
name: bindings ? JSON.stringify(bindings) : undefined,
minLevel: logger.settings.minLevel,
}),
level,
);

View File

@@ -41,6 +41,26 @@ describe("createSubsystemLogger().isEnabled", () => {
expect(log.isEnabled("debug", "file")).toBe(false);
});
it("uses threshold ordering for non-equal console levels", () => {
setLoggerOverride({ level: "silent", consoleLevel: "fatal" });
const fatalOnly = createSubsystemLogger("agent/embedded");
expect(fatalOnly.isEnabled("error", "console")).toBe(false);
expect(fatalOnly.isEnabled("fatal", "console")).toBe(true);
setLoggerOverride({ level: "silent", consoleLevel: "trace" });
const traceLogger = createSubsystemLogger("agent/embedded");
expect(traceLogger.isEnabled("debug", "console")).toBe(true);
});
it("never treats silent as an emittable console level", () => {
setLoggerOverride({ level: "silent", consoleLevel: "info" });
const log = createSubsystemLogger("agent/embedded");
expect(log.isEnabled("silent", "console")).toBe(false);
});
it("returns false when neither console nor file logging would emit", () => {
setLoggerOverride({ level: "silent", consoleLevel: "silent" });
const log = createSubsystemLogger("agent/embedded");

View File

@@ -30,12 +30,15 @@ export type SubsystemLogger = {
};
function shouldLogToConsole(level: LogLevel, settings: { level: LogLevel }): boolean {
if (level === "silent") {
return false;
}
if (settings.level === "silent") {
return false;
}
const current = levelToMinLevel(level);
const min = levelToMinLevel(settings.level);
return current <= min;
return current >= min;
}
type ChalkInstance = InstanceType<typeof Chalk>;