fix(cron): preserve isolated message targets by obviyus · Pull Request #69153 · openclaw/openclaw

obviyus · 2026-04-20T02:02:12Z

Preserves explicit target context for isolated cron runs with delivery.mode: none, and forwards that context into embedded agent messaging.

greptile-apps · 2026-04-20T02:04:49Z

Greptile Summary

This PR fixes a bug where explicit message targets in delivery.mode: none cron jobs were being discarded, and also fixes delivery-plan.ts so that channel only defaults to "last" for announce mode (not none). The hasMessageTargetContext guard in run.ts now correctly gates resolveDeliveryTarget on the presence of explicit to, threadId, or accountId fields — addressing the concern raised in the previous review thread.

Confidence Score: 5/5

Safe to merge — all prior P1 concerns are resolved, logic is correct, and new tests cover both the explicit-target forwarding and the bare-none early-exit path.

The previous thread's core concern (channel defaulting to 'last' making hasMessageTargetContext always true) is addressed by scoping the 'last' default only to announce mode in delivery-plan.ts. The hasMessageTargetContext guard correctly uses to/threadId/accountId rather than channel. Tests are thorough.

No files require special attention.

_{Reviews (2): Last reviewed commit: "test(cron): fix isolated target test typ..." | Re-trigger Greptile}

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4bd5fe7b9a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

obviyus · 2026-04-20T02:19:38Z

@greptile-apps review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c7bd3290a6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

aisle-research-bot · 2026-04-20T02:28:09Z

🔒 Aisle Security Analysis

We found 1 potential security issue(s) in this PR:

#	Severity	Title
1	🟠 High	Cron isolated runs allow messaging target context even when delivery is not requested (mode:none)

1. 🟠 Cron isolated runs allow messaging target context even when delivery is not requested (mode:none)

Property	Value
Severity	High
CWE	CWE-284
Location	`src/cron/isolated-agent/run.ts:144-178`

Description

resolveCronDeliveryContext now resolves and forwards delivery target context to embedded agent runs even when cron delivery is not requested (deliveryPlan.requested === false) as long as any of to, threadId, or accountId is present (and mode is not webhook).

This can enable a control/authorization bypass depending on who can create/modify cron jobs:

For delivery.mode: "none", the embedded run still has the message tool enabled (disableMessageTool: false).
With this change, a job author can supply explicit delivery fields (e.g. to, threadId) and have them propagated into the embedded run as messageTo, messageThreadId, and derived currentChannelId.
Because requireExplicitMessageTarget is only enforced when deliveryRequested is true, the agent run may be able to send messages using these pre-filled defaults without an explicit user-provided target in the prompt.
Additionally, allowing accountId or threadId alone to trigger resolution can cause implicit last-target resolution via session state (channel: "last" with to: undefined), reintroducing implicit routing even when delivery is not requested.

Vulnerable logic (changed behavior):

const hasMessageTargetContext =
  deliveryPlan.mode !== "webhook" &&
  (deliveryPlan.to !== undefined ||
    deliveryPlan.threadId !== undefined ||
    deliveryPlan.accountId !== undefined);
if (!deliveryPlan.requested && !hasMessageTargetContext) {
  // ... skip resolution
}
// otherwise resolves delivery target even when requested=false

If cron definitions are tenant-controlled or editable by less-trusted actors, this enables arbitrary outbound messaging/spam/exfiltration from cron runs by embedding a target in the job definition (or by causing implicit “last target” resolution via session state).

Recommendation

Tighten the conditions under which delivery target context is resolved/forwarded when delivery.mode is none.

Options (pick one based on intended product behavior):

Strict: never resolve/forward any delivery target context unless deliveryPlan.requested === true.
Explicit-only: if you want to allow message-tool defaults when mode:none, require that the target is fully explicit (e.g. to is set), and do not allow accountId or threadId alone to trigger resolution (which can fall back to implicit session targets):

const hasExplicitMessageTarget =
  deliveryPlan.mode === "none" && typeof deliveryPlan.to === "string" && deliveryPlan.to.trim();

if (!deliveryPlan.requested && !hasExplicitMessageTarget) {
  // do not resolve delivery target
}

Additionally, consider enforcing requireExplicitMessageTarget for mode:none when an explicit target is present, to avoid silent/default routing:

requireExplicitMessageTarget: params.resolvedDelivery.ok && (params.deliveryRequested || params.deliveryMode === "none")

Finally, ensure resolveDeliveryTarget (or the message tool) performs authorization/ACL checks for explicit targets in multi-tenant environments (e.g., verify the actor/cron owner can send to the requested to/channel/accountId).

Analyzed PR: #69153 at commit 05a1627

_{Last updated on: 2026-04-20T02:40:05Z}

obviyus · 2026-04-20T02:35:44Z

Landed in 7d9a9d8 via squash merge from 05a1627.\n\n- source: https://github.com/openclaw/openclaw/pull/69153/commits/05a16277f89fd3579a63c0de4fd0763870eae723\n- merge: 7d9a9d8

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 05a16277f8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-04-20T02:36:55Z

+    (deliveryPlan.to !== undefined ||
+      deliveryPlan.threadId !== undefined ||
+      deliveryPlan.accountId !== undefined);


Avoid resolving target for mode=none without explicit recipient

This condition treats threadId or accountId alone as “message target context”, which skips the early return and then calls resolveDeliveryTarget with channel: "last" later in the function; as a result, jobs like delivery: { mode: "none", accountId: "ops" } (or only threadId) can still inherit a session/default recipient and set currentChannelId, allowing agent message calls with no explicit target to go to a stale/default destination. That reintroduces implicit routing for a mode:none configuration that did not provide an explicit recipient.

Useful? React with 👍 / 👎.

* test(cron): cover delivery target context for mode none * fix(cron): preserve target context for delivery mode none * test(cron): cover isolated message target forwarding * fix(cron): forward isolated message targets into embedded runs * fix(cron): ignore implicit last-target context for mode none * fix(cron): keep mode none channel explicit only * test(cron): fix isolated target test typing * fix: preserve isolated message targets (openclaw#69153) * fix: preserve isolated message targets (openclaw#69153)

openclaw-barnacle Bot added size: S maintainer Maintainer-authored PR labels Apr 20, 2026

obviyus self-assigned this Apr 20, 2026

greptile-apps Bot reviewed Apr 20, 2026

View reviewed changes

Comment thread src/cron/isolated-agent/run.ts

chatgpt-codex-connector Bot reviewed Apr 20, 2026

View reviewed changes

Comment thread src/cron/delivery-plan.ts

openclaw-barnacle Bot added size: M and removed size: S labels Apr 20, 2026

obviyus added 7 commits April 20, 2026 07:53

test(cron): cover delivery target context for mode none

d6b7329

fix(cron): preserve target context for delivery mode none

9d33235

test(cron): cover isolated message target forwarding

ac80a4a

fix(cron): forward isolated message targets into embedded runs

0ac87d2

fix(cron): ignore implicit last-target context for mode none

3e9247b

fix(cron): keep mode none channel explicit only

5919117

test(cron): fix isolated target test typing

232656b

chatgpt-codex-connector Bot reviewed Apr 20, 2026

View reviewed changes

Comment thread src/cron/isolated-agent/run-executor.ts Outdated

fix: preserve isolated message targets (openclaw#69153)

bf6ca5e

obviyus force-pushed the fix/cron-isolated-telegram-targeting branch from c7bd329 to bf6ca5e Compare April 20, 2026 02:28

fix: preserve isolated message targets (openclaw#69153)

05a1627

obviyus merged commit 7d9a9d8 into openclaw:main Apr 20, 2026
44 of 47 checks passed

chatgpt-codex-connector Bot reviewed Apr 20, 2026

View reviewed changes

github-actions Bot mentioned this pull request Apr 20, 2026

📡 Upstream Digest — 2026-04-20 04:56 UTC curtismercier/openclaw-mods#621

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(cron): preserve isolated message targets#69153

fix(cron): preserve isolated message targets#69153
obviyus merged 9 commits into
openclaw:mainfrom
obviyus:fix/cron-isolated-telegram-targeting

obviyus commented Apr 20, 2026

Uh oh!

greptile-apps Bot commented Apr 20, 2026 •

edited

Loading

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

obviyus commented Apr 20, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

aisle-research-bot Bot commented Apr 20, 2026 •

edited

Loading

Description

Recommendation

Uh oh!

Uh oh!

obviyus commented Apr 20, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Apr 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

obviyus commented Apr 20, 2026

Uh oh!

greptile-apps Bot commented Apr 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

obviyus commented Apr 20, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

aisle-research-bot Bot commented Apr 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔒 Aisle Security Analysis

Description

Recommendation

Uh oh!

Uh oh!

obviyus commented Apr 20, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

greptile-apps Bot commented Apr 20, 2026 •

edited

Loading

aisle-research-bot Bot commented Apr 20, 2026 •

edited

Loading