fix(heartbeat): force owner downgrade for untrusted hook:wake system events [AI-assisted] (#66031)

* fix: address issue * fix: address PR review feedback * fix: address review-pr skill feedback * fix: address PR review feedback * fix: address PR review feedback * fix: address PR review feedback * fix: address PR review feedback * fix: address PR review feedback * fix: address PR review feedback * docs: add changelog entry for PR merge
2026-05-06 10:40:43 +00:00 · 2026-04-14 00:14:17 +05:30
parent 587e72df4d
commit 31281bc92f
6 changed files with 269 additions and 5 deletions
--- 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.
--- 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<string, unknown>),
+      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<string, unknown>),
+      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<string, unknown>),
+      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:"
--- 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<typeof import("./session-updates.runti
 let sessionStoreRuntimePromise: Promise<
  typeof import("../../config/sessions/store.runtime.js")
 > | 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,
--- 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<ReturnType<typeof runHeartbeatOnce>>;
    sendTelegram: ReturnType<typeof vi.fn>;
-    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 = {
--- 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);
--- 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({