Skip to content

Commit f9f55bb

Browse files
vincentkocvincentkoc
committed
fix(gateway): harden health snapshot exposure
1 parent 38d7f77 commit f9f55bb

13 files changed

Lines changed: 268 additions & 37 deletions

‎CHANGELOG.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ Docs: https://docs.openclaw.ai
231231
- Lobster/Gateway: memoize repeated Ajv schema compilation before loading the embedded Lobster runtime so scheduled workflows and `llm.invoke` loops stop growing gateway heap on content-identical schemas. Fixes #71148. Thanks @cmi525, @vsolaz, and @vincentkoc.
232232
- Codex harness: normalize cached input tokens before session/context accounting so prompt cache reads are not double-counted in `/status`, `session_status`, or persisted `sessionEntry.totalTokens`. Fixes #69298. Thanks @richardmqq.
233233
- Hooks/session-memory: use the host local timezone for memory filenames, fallback timestamp slugs, and markdown headers instead of UTC dates. Fixes #46703. (#46721) Thanks @Astro-Han.
234-
- Gateway health: preserve live runtime-backed channel/account state in `gateway.health` snapshots and cached refreshes, keep probe intent explicit, and retain probe payloads when plugin summaries omit them. (#39921, #42586, #46527, #52770, #42543) Thanks @FAL1989, @rstar327, @0xble, and @ajayr.
234+
- Gateway health: preserve live runtime-backed channel/account state in `gateway.health` snapshots and cached refreshes while keeping raw probe payloads on sensitive/admin paths only. (#39921, #42586, #46527, #52770, #42543) Thanks @FAL1989, @rstar327, @0xble, and @ajayr.
235235
- Feishu: extract quoted/replied interactive-card text across schema 1.0, schema 2.0, i18n, template-variable, and post-format fallback shapes without carrying broad generated/config churn from related parser experiments. (#38776, #60383, #42218, #45936) Thanks @lishuaigit, @lskun, @just2gooo, and @Br1an67.
236236
- Telegram/agents: hide raw failed write/edit warning messages in Telegram when the assistant already explicitly acknowledges the failed action, while keeping warnings when the reply claims success or omits the failure; #39406 remains the broader configurable delivery-policy follow-up. Fixes #51065; covers #39631. Thanks @Bartok9 and @Bortlesboat.
237237
- Exec approvals: accept a symlinked `OPENCLAW_HOME` as the trusted approvals root while still rejecting symlinked `.openclaw` path components below it. (#64663) Thanks @FunJim.

‎src/channels/account-snapshot-fields.test.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,12 +37,25 @@ describe("projectSafeChannelAccountSnapshotFields", () => {
3737

3838
it("preserves non-secret transport liveness timestamps", () => {
3939
const snapshot = projectSafeChannelAccountSnapshotFields({
40+
connected: true,
41+
lastConnectedAt: 123,
4042
lastInboundAt: 123,
43+
lastOutboundAt: 234,
44+
lastMessageAt: null,
45+
lastEventAt: 345,
4146
lastTransportActivityAt: 456,
47+
channelAccessToken: "line-token",
48+
channelSecret: "line-secret", // pragma: allowlist secret
49+
probe: { ok: true, token: "probe-secret" },
4250
});
4351

4452
expect(snapshot).toEqual({
53+
connected: true,
54+
lastConnectedAt: 123,
4555
lastInboundAt: 123,
56+
lastOutboundAt: 234,
57+
lastMessageAt: null,
58+
lastEventAt: 345,
4659
lastTransportActivityAt: 456,
4760
});
4861
});

‎src/channels/account-snapshot-fields.ts‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,16 @@ function readNumber(record: Record<string, unknown>, key: string): number | unde
2626
return typeof value === "number" && Number.isFinite(value) ? value : undefined;
2727
}
2828

29+
function readNullableNumber(
30+
record: Record<string, unknown>,
31+
key: string,
32+
): number | null | undefined {
33+
if (record[key] === null) {
34+
return null;
35+
}
36+
return readNumber(record, key);
37+
}
38+
2939
function readStringArray(record: Record<string, unknown>, key: string): string[] | undefined {
3040
const value = record[key];
3141
if (!Array.isArray(value)) {
@@ -162,6 +172,7 @@ export function projectSafeChannelAccountSnapshotFields(
162172
return {};
163173
}
164174
const name = normalizeOptionalString(record.name);
175+
const statusState = normalizeOptionalString(record.statusState);
165176
const healthState = normalizeOptionalString(record.healthState);
166177
const mode = normalizeOptionalString(record.mode);
167178
const dmPolicy = normalizeOptionalString(record.dmPolicy);
@@ -180,16 +191,39 @@ export function projectSafeChannelAccountSnapshotFields(
180191
...(readBoolean(record, "connected") !== undefined
181192
? { connected: readBoolean(record, "connected") }
182193
: {}),
194+
...(readBoolean(record, "restartPending") !== undefined
195+
? { restartPending: readBoolean(record, "restartPending") }
196+
: {}),
183197
...(readNumber(record, "reconnectAttempts") !== undefined
184198
? { reconnectAttempts: readNumber(record, "reconnectAttempts") }
185199
: {}),
200+
...(readNullableNumber(record, "lastConnectedAt") !== undefined
201+
? { lastConnectedAt: readNullableNumber(record, "lastConnectedAt") }
202+
: {}),
186203
...(readNumber(record, "lastInboundAt") !== undefined
187204
? { lastInboundAt: readNumber(record, "lastInboundAt") }
188205
: {}),
206+
...(readNullableNumber(record, "lastOutboundAt") !== undefined
207+
? { lastOutboundAt: readNullableNumber(record, "lastOutboundAt") }
208+
: {}),
209+
...(readNullableNumber(record, "lastMessageAt") !== undefined
210+
? { lastMessageAt: readNullableNumber(record, "lastMessageAt") }
211+
: {}),
212+
...(readNullableNumber(record, "lastEventAt") !== undefined
213+
? { lastEventAt: readNullableNumber(record, "lastEventAt") }
214+
: {}),
189215
...(readNumber(record, "lastTransportActivityAt") !== undefined
190216
? { lastTransportActivityAt: readNumber(record, "lastTransportActivityAt") }
191217
: {}),
218+
...(statusState ? { statusState } : {}),
192219
...(healthState ? { healthState } : {}),
220+
...(readBoolean(record, "busy") !== undefined ? { busy: readBoolean(record, "busy") } : {}),
221+
...(readNumber(record, "activeRuns") !== undefined
222+
? { activeRuns: readNumber(record, "activeRuns") }
223+
: {}),
224+
...(readNullableNumber(record, "lastRunActivityAt") !== undefined
225+
? { lastRunActivityAt: readNullableNumber(record, "lastRunActivityAt") }
226+
: {}),
193227
...(mode ? { mode } : {}),
194228
...(dmPolicy ? { dmPolicy } : {}),
195229
...(readStringArray(record, "allowFrom")

‎src/commands/health.snapshot.test.ts‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -477,6 +477,69 @@ describe("getHealthSnapshot", () => {
477477
expect(telegram.accounts?.default?.probe?.ok).toBe(true);
478478
});
479479

480+
it("omits secret runtime fields and raw probe payloads from non-sensitive health snapshots", async () => {
481+
testConfig = { channels: { telegram: { botToken: "t-1" } } };
482+
testStore = {};
483+
vi.stubEnv("DISCORD_BOT_TOKEN", "");
484+
buildTelegramHealthSummaryForTest = (snapshot) => ({
485+
accountId: snapshot.accountId,
486+
configured: Boolean(snapshot.configured),
487+
probe: { ok: true, token: "summary-secret" },
488+
});
489+
probeTelegramAccountForTestOverride = async () => ({
490+
ok: true,
491+
bot: { username: "runtime_bot" },
492+
token: "probe-secret",
493+
});
494+
495+
const snap = await getHealthSnapshot({
496+
timeoutMs: 25,
497+
includeSensitive: false,
498+
runtimeSnapshot: {
499+
channels: {
500+
telegram: {
501+
accountId: "default",
502+
connected: true,
503+
lastConnectedAt: 123,
504+
channelAccessToken: "line-token",
505+
channelSecret: "line-secret", // pragma: allowlist secret
506+
webhookUrl: "https://example.test/hook?secret=1",
507+
},
508+
},
509+
channelAccounts: {},
510+
},
511+
});
512+
const telegram = snap.channels.telegram as {
513+
connected?: boolean;
514+
lastConnectedAt?: number;
515+
probe?: unknown;
516+
channelAccessToken?: string;
517+
channelSecret?: string;
518+
webhookUrl?: string;
519+
accounts?: Record<
520+
string,
521+
{
522+
connected?: boolean;
523+
lastConnectedAt?: number;
524+
probe?: unknown;
525+
channelAccessToken?: string;
526+
channelSecret?: string;
527+
webhookUrl?: string;
528+
}
529+
>;
530+
};
531+
532+
expect(telegram.connected).toBe(true);
533+
expect(telegram.lastConnectedAt).toBe(123);
534+
expect(telegram.probe).toBeUndefined();
535+
expect(telegram.channelAccessToken).toBeUndefined();
536+
expect(telegram.channelSecret).toBeUndefined();
537+
expect(telegram.webhookUrl).toBeUndefined();
538+
expect(telegram.accounts?.default?.connected).toBe(true);
539+
expect(telegram.accounts?.default?.probe).toBeUndefined();
540+
expect(telegram.accounts?.default?.channelAccessToken).toBeUndefined();
541+
});
542+
480543
it("returns structured telegram probe errors", async () => {
481544
testConfig = { channels: { telegram: { botToken: "bad-token" } } };
482545
testStore = {};

‎src/commands/health.ts‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { resolveDefaultAgentId } from "../agents/agent-scope.js";
2+
import { projectSafeChannelAccountSnapshotFields } from "../channels/account-snapshot-fields.js";
23
import { resolveChannelDefaultAccountId } from "../channels/plugins/helpers.js";
34
import { listReadOnlyChannelPluginsForConfig } from "../channels/plugins/read-only.js";
45
import type { ChannelPlugin } from "../channels/plugins/types.plugin.js";
@@ -256,6 +257,7 @@ async function resolveHealthAccountContext(params: {
256257
export async function getHealthSnapshot(params?: {
257258
timeoutMs?: number;
258259
probe?: boolean;
260+
includeSensitive?: boolean;
259261
runtimeSnapshot?: ChannelRuntimeSnapshot;
260262
}): Promise<HealthSummary> {
261263
const timeoutMs = params?.timeoutMs;
@@ -287,6 +289,7 @@ export async function getHealthSnapshot(params?: {
287289
const start = Date.now();
288290
const cappedTimeout = timeoutMs === undefined ? DEFAULT_TIMEOUT_MS : Math.max(50, timeoutMs);
289291
const doProbe = params?.probe !== false;
292+
const includeSensitive = params?.includeSensitive !== false;
290293
const channels: Record<string, ChannelHealthSummary> = {};
291294
const plugins = listReadOnlyChannelPluginsForConfig(cfg, {
292295
includeSetupRuntimeFallback: false,
@@ -368,12 +371,12 @@ export async function getHealthSnapshot(params?: {
368371
params?.runtimeSnapshot?.channelAccounts[plugin.id]?.[accountId] ??
369372
(accountId === defaultAccountId ? params?.runtimeSnapshot?.channels[plugin.id] : undefined);
370373
const snapshot: ChannelAccountSnapshot = {
371-
...runtimeSnapshot,
374+
...projectSafeChannelAccountSnapshotFields(runtimeSnapshot),
372375
accountId,
373376
enabled,
374377
configured,
375378
};
376-
if (probe !== undefined) {
379+
if (includeSensitive && probe !== undefined) {
377380
snapshot.probe = probe;
378381
}
379382
if (lastProbeAt) {
@@ -392,17 +395,19 @@ export async function getHealthSnapshot(params?: {
392395
summary && typeof summary === "object"
393396
? ({ ...snapshot, ...summary } as ChannelAccountHealthSummary)
394397
: ({
398+
...snapshot,
395399
accountId,
396400
configured,
397-
probe,
398-
lastProbeAt,
399401
} satisfies ChannelAccountHealthSummary);
400402
if (record.configured === undefined) {
401403
record.configured = configured;
402404
}
403-
if (record.probe === undefined && probe !== undefined) {
405+
if (includeSensitive && record.probe === undefined && probe !== undefined) {
404406
record.probe = probe;
405407
}
408+
if (!includeSensitive) {
409+
delete record.probe;
410+
}
406411
if (record.lastProbeAt === undefined && lastProbeAt) {
407412
record.lastProbeAt = lastProbeAt;
408413
}

‎src/gateway/server-maintenance.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,10 @@ export function startGatewayMaintenanceTimers(params: {
2626
nodeSendToAllSubscribed: (event: string, payload: unknown) => void;
2727
getPresenceVersion: () => number;
2828
getHealthVersion: () => number;
29-
refreshGatewayHealthSnapshot: (opts?: { probe?: boolean }) => Promise<HealthSummary>;
29+
refreshGatewayHealthSnapshot: (opts?: {
30+
probe?: boolean;
31+
includeSensitive?: boolean;
32+
}) => Promise<HealthSummary>;
3033
logHealth: { error: (msg: string) => void };
3134
dedupe: Map<string, DedupeEntry>;
3235
chatAbortControllers: Map<string, ChatAbortControllerEntry>;

‎src/gateway/server-methods/health.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,20 +8,22 @@ import type { GatewayRequestHandlers } from "./types.js";
88
const ADMIN_SCOPE = "operator.admin";
99

1010
export const healthHandlers: GatewayRequestHandlers = {
11-
health: async ({ respond, context, params }) => {
11+
health: async ({ respond, context, params, client }) => {
1212
const { getHealthCache, refreshHealthSnapshot, logHealth } = context;
1313
const wantsProbe = params?.probe === true;
14+
const scopes = Array.isArray(client?.connect?.scopes) ? client.connect.scopes : [];
15+
const includeSensitive = scopes.includes(ADMIN_SCOPE);
1416
const now = Date.now();
1517
const cached = getHealthCache();
1618
if (!wantsProbe && cached && now - cached.ts < HEALTH_REFRESH_INTERVAL_MS) {
1719
respond(true, cached, undefined, { cached: true });
18-
void refreshHealthSnapshot({ probe: false }).catch((err) =>
20+
void refreshHealthSnapshot({ probe: false, includeSensitive }).catch((err) =>
1921
logHealth.error(`background health refresh failed: ${formatError(err)}`),
2022
);
2123
return;
2224
}
2325
try {
24-
const snap = await refreshHealthSnapshot({ probe: wantsProbe });
26+
const snap = await refreshHealthSnapshot({ probe: wantsProbe, includeSensitive });
2527
respond(true, snap, undefined);
2628
} catch (err) {
2729
respond(false, undefined, errorShape(ErrorCodes.UNAVAILABLE, formatForLog(err)));

‎src/gateway/server-methods/shared-types.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,10 @@ export type GatewayRequestContext = {
4646
pluginApprovalManager?: ExecApprovalManager<PluginApprovalRequestPayload>;
4747
loadGatewayModelCatalog: () => Promise<ModelCatalogEntry[]>;
4848
getHealthCache: () => HealthSummary | null;
49-
refreshHealthSnapshot: (opts?: { probe?: boolean }) => Promise<HealthSummary>;
49+
refreshHealthSnapshot: (opts?: {
50+
probe?: boolean;
51+
includeSensitive?: boolean;
52+
}) => Promise<HealthSummary>;
5053
logHealth: { error: (message: string) => void };
5154
logGateway: SubsystemLogger;
5255
incrementPresenceVersion: () => number;

‎src/gateway/server-node-events-types.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,10 @@ export type NodeEventContext = {
2525
dedupe: Map<string, DedupeEntry>;
2626
agentRunSeq: Map<string, number>;
2727
getHealthCache: () => HealthSummary | null;
28-
refreshHealthSnapshot: (opts?: { probe?: boolean }) => Promise<HealthSummary>;
28+
refreshHealthSnapshot: (opts?: {
29+
probe?: boolean;
30+
includeSensitive?: boolean;
31+
}) => Promise<HealthSummary>;
2932
loadGatewayModelCatalog: () => Promise<ModelCatalogEntry[]>;
3033
logGateway: { warn: (msg: string) => void };
3134
};

‎src/gateway/server/health-state.test.ts‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ describe("refreshGatewayHealthSnapshot", () => {
5454
expect(getHealthSnapshotMock).toHaveBeenCalledTimes(1);
5555
expect(getHealthSnapshotMock).toHaveBeenCalledWith({
5656
probe: false,
57+
includeSensitive: false,
5758
runtimeSnapshot: undefined,
5859
});
5960
resolveSnapshot?.(createHealthSummary());
@@ -84,7 +85,71 @@ describe("refreshGatewayHealthSnapshot", () => {
8485
.map((call) => call[0]?.probe)
8586
.toSorted((a, b) => Number(a) - Number(b)),
8687
).toEqual([false, true]);
88+
expect(getHealthSnapshotMock.mock.calls.map((call) => call[0]?.includeSensitive)).toEqual([
89+
false,
90+
false,
91+
]);
8792
expect(getHealthSnapshotMock.mock.calls[0]?.[0]?.runtimeSnapshot).toBe(runtimeSnapshot);
8893
expect(getHealthSnapshotMock.mock.calls[1]?.[0]?.runtimeSnapshot).toBeUndefined();
8994
});
95+
96+
it("does not cache or broadcast sensitive health refreshes", async () => {
97+
const healthState = await loadHealthState();
98+
const sensitiveSummary = createHealthSummary();
99+
const safeSummary = createHealthSummary();
100+
const broadcast = vi.fn();
101+
getHealthSnapshotMock.mockResolvedValueOnce(sensitiveSummary).mockResolvedValueOnce(safeSummary);
102+
healthState.setBroadcastHealthUpdate(broadcast);
103+
const version = healthState.getHealthVersion();
104+
105+
await healthState.refreshGatewayHealthSnapshot({ probe: true, includeSensitive: true });
106+
107+
expect(healthState.getHealthCache()).toBeNull();
108+
expect(healthState.getHealthVersion()).toBe(version);
109+
expect(broadcast).not.toHaveBeenCalled();
110+
111+
await healthState.refreshGatewayHealthSnapshot({ probe: false });
112+
113+
expect(healthState.getHealthCache()).toBe(safeSummary);
114+
expect(healthState.getHealthVersion()).toBe(version + 1);
115+
expect(broadcast).toHaveBeenCalledWith(safeSummary);
116+
});
117+
118+
it("keeps sensitive and public refreshes on separate in-flight promises", async () => {
119+
const healthState = await loadHealthState();
120+
const sensitiveSummary = createHealthSummary();
121+
const safeSummary = createHealthSummary();
122+
let resolveSensitive: (() => void) | undefined;
123+
let resolveSafe: (() => void) | undefined;
124+
getHealthSnapshotMock
125+
.mockImplementationOnce(
126+
() =>
127+
new Promise<HealthSummary>((resolve) => {
128+
resolveSensitive = () => resolve(sensitiveSummary);
129+
}),
130+
)
131+
.mockImplementationOnce(
132+
() =>
133+
new Promise<HealthSummary>((resolve) => {
134+
resolveSafe = () => resolve(safeSummary);
135+
}),
136+
);
137+
138+
const sensitive = healthState.refreshGatewayHealthSnapshot({
139+
probe: true,
140+
includeSensitive: true,
141+
});
142+
const safe = healthState.refreshGatewayHealthSnapshot({ probe: false });
143+
144+
expect(getHealthSnapshotMock).toHaveBeenCalledTimes(2);
145+
expect(getHealthSnapshotMock.mock.calls[0]?.[0]?.includeSensitive).toBe(true);
146+
expect(getHealthSnapshotMock.mock.calls[1]?.[0]?.includeSensitive).toBe(false);
147+
148+
resolveSensitive?.();
149+
resolveSafe?.();
150+
151+
await expect(sensitive).resolves.toBe(sensitiveSummary);
152+
await expect(safe).resolves.toBe(safeSummary);
153+
expect(healthState.getHealthCache()).toBe(safeSummary);
154+
});
90155
});

0 commit comments

Comments
 (0)