diff --git a/CHANGELOG.md b/CHANGELOG.md index 33b272c73ce..b669bf91e35 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- fix(heartbeat): force owner downgrade for untrusted hook:wake system events [AI-assisted]. (#66031) Thanks @pgondhi987. - fix(browser): enforce SSRF policy on snapshot, screenshot, and tab routes [AI]. (#66040) Thanks @pgondhi987. - fix(msteams): enforce sender allowlist checks on SSO signin invokes [AI]. (#66033) Thanks @pgondhi987. - fix(config): redact sourceConfig and runtimeConfig alias fields in redactConfigSnapshot [AI]. (#66030) Thanks @pgondhi987. diff --git a/src/auto-reply/reply/get-reply-run.media-only.test.ts b/src/auto-reply/reply/get-reply-run.media-only.test.ts index 7f5047aacbb..ef5f86c4940 100644 --- a/src/auto-reply/reply/get-reply-run.media-only.test.ts +++ b/src/auto-reply/reply/get-reply-run.media-only.test.ts @@ -712,6 +712,52 @@ describe("runPreparedReply media-only handling", () => { expect(call?.followupRun.run.extraSystemPrompt ?? "").not.toContain("Runtime System Events"); }); + it("downgrades sender ownership when drained system events include untrusted lines", async () => { + vi.mocked(drainFormattedSystemEvents).mockResolvedValueOnce( + "System (untrusted): [t] External webhook payload.", + ); + const params = baseParams(); + params.command = { + ...(params.command as Record), + senderIsOwner: true, + } as never; + + await runPreparedReply(params); + + const call = vi.mocked(runReplyAgent).mock.calls[0]?.[0]; + expect(call?.followupRun.run.senderIsOwner).toBe(false); + }); + + it("keeps sender ownership when drained system events are trusted", async () => { + vi.mocked(drainFormattedSystemEvents).mockResolvedValueOnce("System: [t] Trusted event."); + const params = baseParams(); + params.command = { + ...(params.command as Record), + senderIsOwner: true, + } as never; + + await runPreparedReply(params); + + const call = vi.mocked(runReplyAgent).mock.calls[0]?.[0]; + expect(call?.followupRun.run.senderIsOwner).toBe(true); + }); + + it("does not downgrade sender ownership when trusted event text contains the untrusted marker", async () => { + vi.mocked(drainFormattedSystemEvents).mockResolvedValueOnce( + "System: [t] Relay text mentions System (untrusted): but event is trusted.", + ); + const params = baseParams(); + params.command = { + ...(params.command as Record), + senderIsOwner: true, + } as never; + + await runPreparedReply(params); + + const call = vi.mocked(runReplyAgent).mock.calls[0]?.[0]; + expect(call?.followupRun.run.senderIsOwner).toBe(true); + }); + it("preserves first-token think hint when system events are prepended", async () => { // drainFormattedSystemEvents returns just the events block; the caller prepends it. // The hint must be extracted from the user body BEFORE prepending, so "System:" diff --git a/src/auto-reply/reply/get-reply-run.ts b/src/auto-reply/reply/get-reply-run.ts index 9e702ed67f3..7c249a46b4a 100644 --- a/src/auto-reply/reply/get-reply-run.ts +++ b/src/auto-reply/reply/get-reply-run.ts @@ -97,6 +97,7 @@ let sessionUpdatesRuntimePromise: Promise | null = null; +const UNTRUSTED_SYSTEM_EVENT_LINE_RE = /^System \(untrusted\):/m; function loadPiEmbeddedRuntime() { piEmbeddedRuntimePromise ??= import("../../agents/pi-embedded.runtime.js"); @@ -392,6 +393,7 @@ export async function runPreparedReply( ? `[Thread starter - for context]\n${threadStarterBody}` : undefined; const drainedSystemEventBlocks: string[] = []; + let forceSenderIsOwnerFalseFromSystemEvents = false; const rebuildPromptBodies = async (): Promise<{ prefixedCommandBody: string; queuedBody: string; @@ -405,6 +407,9 @@ export async function runPreparedReply( }); if (eventsBlock) { drainedSystemEventBlocks.push(eventsBlock); + if (UNTRUSTED_SYSTEM_EVENT_LINE_RE.test(eventsBlock)) { + forceSenderIsOwnerFalseFromSystemEvents = true; + } } } return buildReplyPromptBodies({ @@ -628,7 +633,7 @@ export async function runPreparedReply( senderName: normalizeOptionalString(sessionCtx.SenderName), senderUsername: normalizeOptionalString(sessionCtx.SenderUsername), senderE164: normalizeOptionalString(sessionCtx.SenderE164), - senderIsOwner: command.senderIsOwner, + senderIsOwner: forceSenderIsOwnerFalseFromSystemEvents ? false : command.senderIsOwner, sessionFile: preparedSessionState.sessionFile, workspaceDir, config: cfg, diff --git a/src/infra/heartbeat-runner.ghost-reminder.test.ts b/src/infra/heartbeat-runner.ghost-reminder.test.ts index 7a29bf3c6b9..7d29c99e0d2 100644 --- a/src/infra/heartbeat-runner.ghost-reminder.test.ts +++ b/src/infra/heartbeat-runner.ghost-reminder.test.ts @@ -34,6 +34,7 @@ describe("Ghost reminder bug (issue #13317)", () => { tmpDir: string; storePath: string; target?: "telegram" | "none"; + isolatedSession?: boolean; }): Promise<{ cfg: OpenClawConfig; sessionKey: string }> => { const cfg: OpenClawConfig = { agents: { @@ -42,6 +43,7 @@ describe("Ghost reminder bug (issue #13317)", () => { heartbeat: { every: "5m", target: params.target ?? "telegram", + ...(params.isolatedSession === true ? { isolatedSession: true } : {}), }, }, }, @@ -94,10 +96,16 @@ describe("Ghost reminder bug (issue #13317)", () => { reason: string; enqueue: (sessionKey: string) => void; target?: "telegram" | "none"; + isolatedSession?: boolean; }): Promise<{ result: Awaited>; sendTelegram: ReturnType; - calledCtx: { Provider?: string; Body?: string; ForceSenderIsOwnerFalse?: boolean } | null; + calledCtx: { + Provider?: string; + Body?: string; + SessionKey?: string; + ForceSenderIsOwnerFalse?: boolean; + } | null; replyCallCount: number; }> => { return withTempHeartbeatSandbox( @@ -107,6 +115,7 @@ describe("Ghost reminder bug (issue #13317)", () => { tmpDir, storePath, target: params.target, + isolatedSession: params.isolatedSession, }); params.enqueue(sessionKey); const result = await runHeartbeatOnce({ @@ -121,6 +130,8 @@ describe("Ghost reminder bug (issue #13317)", () => { const calledCtx = (getReplySpy.mock.calls[0]?.[0] ?? null) as { Provider?: string; Body?: string; + SessionKey?: string; + ForceSenderIsOwnerFalse?: boolean; } | null; return { result, @@ -281,6 +292,127 @@ describe("Ghost reminder bug (issue #13317)", () => { expect(sendTelegram).not.toHaveBeenCalled(); }); + it("classifies hook:wake exec completions as exec-event prompts", async () => { + const { result, sendTelegram, calledCtx } = await runHeartbeatCase({ + tmpPrefix: "openclaw-hook-exec-", + replyText: "Handled internally", + reason: "hook:wake", + target: "none", + enqueue: (sessionKey) => { + enqueueSystemEvent("exec finished: webhook-triggered backup completed", { sessionKey }); + }, + }); + + expect(result.status).toBe("ran"); + expect(calledCtx?.Provider).toBe("exec-event"); + expect(calledCtx?.ForceSenderIsOwnerFalse).toBe(true); + expect(calledCtx?.Body).toContain("Handle the result internally"); + expect(sendTelegram).not.toHaveBeenCalled(); + }); + + it("does not classify base-session hook:wake exec completions as exec-event prompts when isolated sessions are enabled", async () => { + const { result, sendTelegram, calledCtx } = await runHeartbeatCase({ + tmpPrefix: "openclaw-hook-exec-isolated-", + replyText: "Handled internally", + reason: "hook:wake", + target: "none", + isolatedSession: true, + enqueue: (sessionKey) => { + enqueueSystemEvent("exec finished: webhook-triggered backup completed", { sessionKey }); + }, + }); + + expect(result.status).toBe("ran"); + expect(calledCtx?.Provider).toBe("heartbeat"); + expect(calledCtx?.SessionKey).toContain(":heartbeat"); + expect(calledCtx?.ForceSenderIsOwnerFalse).toBe(false); + expect(sendTelegram).not.toHaveBeenCalled(); + }); + + it("forces owner downgrade for untrusted hook:wake system events", async () => { + const { result, sendTelegram, calledCtx } = await runHeartbeatCase({ + tmpPrefix: "openclaw-hook-untrusted-", + replyText: "Handled internally", + reason: "hook:wake", + target: "none", + enqueue: (sessionKey) => { + enqueueSystemEvent("GitHub issue opened: untrusted webhook content", { + sessionKey, + trusted: false, + }); + }, + }); + + expect(result.status).toBe("ran"); + expect(calledCtx?.Provider).toBe("heartbeat"); + expect(calledCtx?.ForceSenderIsOwnerFalse).toBe(true); + expect(sendTelegram).not.toHaveBeenCalled(); + }); + + it("forces owner downgrade for untrusted interval events", async () => { + const { result, sendTelegram, calledCtx } = await runHeartbeatCase({ + tmpPrefix: "openclaw-interval-untrusted-", + replyText: "Handled internally", + reason: "interval", + target: "none", + enqueue: (sessionKey) => { + enqueueSystemEvent("GitHub issue opened: untrusted webhook content", { + sessionKey, + trusted: false, + }); + }, + }); + + expect(result.status).toBe("ran"); + expect(calledCtx?.Provider).toBe("heartbeat"); + expect(calledCtx?.ForceSenderIsOwnerFalse).toBe(true); + expect(sendTelegram).not.toHaveBeenCalled(); + }); + + it("does not force owner downgrade for untrusted hook:wake events with isolated sessions", async () => { + const { result, sendTelegram, calledCtx } = await runHeartbeatCase({ + tmpPrefix: "openclaw-hook-untrusted-isolated-", + replyText: "Handled internally", + reason: "hook:wake", + target: "none", + isolatedSession: true, + enqueue: (sessionKey) => { + enqueueSystemEvent("GitHub issue opened: untrusted webhook content", { + sessionKey, + trusted: false, + }); + }, + }); + + expect(result.status).toBe("ran"); + expect(calledCtx?.Provider).toBe("heartbeat"); + expect(calledCtx?.SessionKey).toContain(":heartbeat"); + expect(calledCtx?.ForceSenderIsOwnerFalse).toBe(false); + expect(sendTelegram).not.toHaveBeenCalled(); + }); + + it("does not force owner downgrade for isolated interval runs with only base-session untrusted events", async () => { + const { result, sendTelegram, calledCtx } = await runHeartbeatCase({ + tmpPrefix: "openclaw-interval-untrusted-isolated-", + replyText: "Handled internally", + reason: "interval", + target: "none", + isolatedSession: true, + enqueue: (sessionKey) => { + enqueueSystemEvent("GitHub issue opened: untrusted webhook content", { + sessionKey, + trusted: false, + }); + }, + }); + + expect(result.status).toBe("ran"); + expect(calledCtx?.Provider).toBe("heartbeat"); + expect(calledCtx?.SessionKey).toContain(":heartbeat"); + expect(calledCtx?.ForceSenderIsOwnerFalse).toBe(false); + expect(sendTelegram).not.toHaveBeenCalled(); + }); + it("routes wake-triggered heartbeat replies using queued system-event delivery context", async () => { await withTempHeartbeatSandbox(async ({ tmpDir, storePath, replySpy }) => { const cfg: OpenClawConfig = { diff --git a/src/infra/heartbeat-runner.isolated-key-stability.test.ts b/src/infra/heartbeat-runner.isolated-key-stability.test.ts index 7aa4b8e1e36..0376f263342 100644 --- a/src/infra/heartbeat-runner.isolated-key-stability.test.ts +++ b/src/infra/heartbeat-runner.isolated-key-stability.test.ts @@ -292,6 +292,51 @@ describe("runHeartbeatOnce – isolated session key stability (#59493)", () => { }); }); + it("classifies hook:wake exec events when they are queued on the active isolated session", async () => { + await withTempHeartbeatSandbox(async ({ tmpDir, storePath }) => { + const cfg = makeIsolatedHeartbeatConfig(tmpDir, storePath); + const baseSessionKey = resolveMainSessionKey(cfg); + const isolatedSessionKey = `${baseSessionKey}:heartbeat`; + await fs.writeFile( + storePath, + JSON.stringify({ + [isolatedSessionKey]: { + sessionId: "sid", + updatedAt: 1, + lastChannel: "whatsapp", + lastProvider: "whatsapp", + lastTo: "+1555", + heartbeatIsolatedBaseSessionKey: baseSessionKey, + }, + }), + "utf-8", + ); + enqueueSystemEvent("exec finished: deploy succeeded", { sessionKey: isolatedSessionKey }); + const replySpy = vi.spyOn(replyModule, "getReplyFromConfig"); + replySpy.mockResolvedValue({ text: "Handled internally" }); + + const result = await runHeartbeatOnce({ + cfg, + sessionKey: isolatedSessionKey, + reason: "hook:wake", + deps: { + getQueueSize: () => 0, + nowMs: () => 0, + }, + }); + + expect(result.status).toBe("ran"); + const calledCtx = replySpy.mock.calls[0]?.[0] as { + SessionKey?: string; + Provider?: string; + ForceSenderIsOwnerFalse?: boolean; + }; + expect(calledCtx.SessionKey).toBe(isolatedSessionKey); + expect(calledCtx.Provider).toBe("exec-event"); + expect(calledCtx.ForceSenderIsOwnerFalse).toBe(true); + }); + }); + it("keeps a forced real :heartbeat session distinct from the heartbeat-isolated sibling", async () => { await withTempHeartbeatSandbox(async ({ tmpDir, storePath }) => { const cfg = makeIsolatedHeartbeatConfig(tmpDir, storePath); diff --git a/src/infra/heartbeat-runner.ts b/src/infra/heartbeat-runner.ts index 36e512b63aa..8259076e342 100644 --- a/src/infra/heartbeat-runner.ts +++ b/src/infra/heartbeat-runner.ts @@ -537,8 +537,28 @@ async function resolveHeartbeatPreflight(params: { const hasTaggedCronEvents = pendingEventEntries.some((event) => event.contextKey?.startsWith("cron:"), ); + // Wake-triggered runs should only inspect pending events when preflight peeks + // the same queue that the run itself will execute/drain. + const shouldInspectWakePendingEvents = (() => { + if (!reasonFlags.isWakeReason) { + return false; + } + if (params.heartbeat?.isolatedSession !== true) { + return true; + } + const configuredSession = resolveHeartbeatSession(params.cfg, params.agentId, params.heartbeat); + const { isolatedSessionKey } = resolveIsolatedHeartbeatSessionKey({ + sessionKey: session.sessionKey, + configuredSessionKey: configuredSession.sessionKey, + sessionEntry: session.entry, + }); + return isolatedSessionKey === session.sessionKey; + })(); const shouldInspectPendingEvents = - reasonFlags.isExecEventReason || reasonFlags.isCronEventReason || hasTaggedCronEvents; + reasonFlags.isExecEventReason || + reasonFlags.isCronEventReason || + shouldInspectWakePendingEvents || + hasTaggedCronEvents; const shouldBypassFileGates = reasonFlags.isExecEventReason || reasonFlags.isCronEventReason || @@ -805,7 +825,11 @@ export async function runHeartbeatOnce(opts: { // If no tasks are due, skip heartbeat entirely if (prompt === null) { - if (preflight.shouldInspectPendingEvents && preflight.pendingEventEntries.length > 0) { + // Wake-triggered events should stay queued when the run short-circuits: + // no reply turn ran, so there is nothing that actually consumed that wake payload. + const shouldConsumeInspectedEvents = + !preflight.isWakeReason && preflight.shouldInspectPendingEvents; + if (shouldConsumeInspectedEvents && preflight.pendingEventEntries.length > 0) { consumeSystemEventEntries(sessionKey, preflight.pendingEventEntries); } return { status: "skipped", reason: "no-tasks-due" }; @@ -868,6 +892,17 @@ export async function runHeartbeatOnce(opts: { } runSessionKey = isolatedSessionKey; } + const activeSessionPendingEventEntries = + runSessionKey === sessionKey + ? preflight.pendingEventEntries + : peekSystemEventEntries(runSessionKey); + const hasUntrustedInspectedEvents = + preflight.shouldInspectPendingEvents && + preflight.pendingEventEntries.some((event) => event.trusted === false); + const hasUntrustedActiveSessionEvents = activeSessionPendingEventEntries.some( + (event) => event.trusted === false, + ); + const hasUntrustedPendingEvents = hasUntrustedInspectedEvents || hasUntrustedActiveSessionEvents; // Update task last run times AFTER successful heartbeat completion const updateTaskTimestamps = async () => { @@ -917,7 +952,7 @@ export async function runHeartbeatOnce(opts: { MessageThreadId: delivery.threadId, Provider: hasExecCompletion ? "exec-event" : hasCronEvents ? "cron-event" : "heartbeat", SessionKey: runSessionKey, - ForceSenderIsOwnerFalse: hasExecCompletion, + ForceSenderIsOwnerFalse: hasExecCompletion || hasUntrustedPendingEvents, }; if (!visibility.showAlerts && !visibility.showOk && !visibility.useIndicator) { emitHeartbeatEvent({