Greptile Summary

This PR adds a narrow server-side allowlist mechanism (ownerOnlyToolAllowlist) that lets isolated cron jobs expose the cron tool to non-owner runs when explicitly included in the job's toolsAllow list. The implementation is well-designed: the allowlist is strictly controlled server-side, the grant is limited to ["cron"] only (no other owner-only tools can be escalated via the cron path), and the existing defense-in-depth pattern (guard wrap + filter) is preserved. Tests cover the happy path, the rejection path, and the end-to-end createOpenClawCodingTools behavior.

Confidence Score: 4/5

Safe to merge; only a minor style inconsistency with tool name normalization was found.

All findings are P2. The security model is sound (allowlist is server-controlled, narrowly scoped to cron, and backed by defense-in-depth). No logic errors or data-correctness issues were found.

src/cron/isolated-agent/run-executor.ts — minor normalization inconsistency in resolveCronOwnerOnlyToolAllowlist.

This is a comment left during a code review.
Path: src/cron/isolated-agent/run-executor.ts
Line: 51

Comment:
**Inconsistent tool name normalization**

`resolveCronOwnerOnlyToolAllowlist` normalizes with `.trim().toLowerCase()`, while the rest of the codebase (including `applyOwnerOnlyToolPolicy`) uses `normalizeToolName`, which also applies `TOOL_NAME_ALIASES`. The two paths are currently equivalent for `"cron"` (no alias exists), but using a different normalization function is inconsistent and could silently diverge if an alias were ever added.

Consider using the shared utility for the comparison as well:

```ts
function resolveCronOwnerOnlyToolAllowlist(toolsAllow: string[] | undefined): string[] | undefined {
  if (!toolsAllow?.some((toolName) => normalizeToolName(toolName) === "cron")) {
    return undefined;
  }
  return ["cron"];
}
```

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "fix: allow cron self-removal in isolated..." | Re-trigger Greptile}

Codex review: keeping this open for maintainer follow-up; there is still a little grit to resolve.

Keep this PR open. The live PR still has the protected maintainer label, and the provided context identifies it as maintainer-authored, so it needs explicit maintainer judgment rather than automated cleanup. Current main also does not already implement the intended cron self-removal behavior: isolated cron runs still execute as non-owner without a narrow cron owner-only grant, owner-only filtering still drops all owner-only tools for non-owners, the cron tool lacks a self-remove-only scope, and cron outcome finalization still discards successful runs when the job removed itself during execution.

Best possible solution:

Keep this PR open for maintainer review. The best path is to finish the security review, confirm the normalized cron allowlist plus self-remove-only guard is the desired authorization model, verify the targeted tests and changed gate, then land or revise the PR. Current main does not already implement the intended behavior, and the related cron reports in the provided context are adjacent rather than canonical superseding items.

What I checked:

• Protected maintainer handling: Live PR metadata shows the PR is open, unmerged, and labeled maintainer; the provided review context also identifies the author association as MEMBER. Repository cleanup policy keeps protected-label and maintainer-authored items open for explicit maintainer handling. (fd2625a16252)
• Current main lacks the isolated cron owner-only grant: createCronPromptExecutor passes senderIsOwner: false and forwards toolsAllow, but there is no ownerOnlyToolAllowlist or equivalent runtime grant in the embedded isolated cron call path. (src/cron/isolated-agent/run-executor.ts:166, fd2625a16252)
• Current owner-only policy filters all owner-only tools for non-owners: applyOwnerOnlyToolPolicy accepts only tools and senderIsOwner; for non-owner sessions it wraps owner-only tools and then filters every owner-only tool out, with no allowlist exception for cron. (src/agents/tool-policy.ts:54, fd2625a16252)
• Current agent tool creation has no scoped cron self-removal option: createOpenClawCodingTools exposes only senderIsOwner for owner-only filtering and passes senderIsOwner into createOpenClawTools; it has no ownerOnlyToolAllowlist, jobId-based cron scope, or cronSelfRemoveOnlyJobId path on current main. (src/agents/pi-tools.ts:341, fd2625a16252)
• Current cron tool lacks a self-remove-only guard: CronToolOptions only carries agentSessionKey and currentDeliveryContext, and the execute path reads action then dispatches without checking a selfRemoveOnlyJobId or restricting actions to remove for the active job. (src/agents/tools/cron-tool.ts:314, fd2625a16252)
• Current cron timer discards removed-job outcomes: When applyOutcomeToStoredJob reloads and cannot find the job, current main logs job not found after forceReload, result discarded and returns. The successful self-removal finalization path proposed by the PR is not present. (src/cron/service/timer.ts:644, fd2625a16252)

Likely related people:

• steipete (Peter Steinberger steipete@gmail.com): Path history shows Peter introduced the centralized owner-only tool gating and has the heaviest recent maintenance history across src/agents/tool-policy.ts, src/cron/service/timer.ts, and isolated cron executor work relevant to this PR. (role: security/cron maintainer; confidence: high; commits: 3d7ad1cfca4d, 729147dcb523, 7d74c29dcc07; files: src/agents/tool-policy.ts, src/cron/isolated-agent/run-executor.ts, src/cron/service/timer.ts)
• jalehman (Josh Lehman josh@martian.engineering): Beyond authoring this PR, Josh has prior merged history in the same isolated cron executor path, including context-engine session key and Telegram topic delivery fixes that sit near the behavior this PR changes. (role: recent isolated cron maintainer; confidence: high; commits: a3c51f91c58a, 8ba82534e6c9; files: src/cron/isolated-agent/run-executor.ts)
• obviyus (Ayaan Zaidi hi@obviy.us): Recent path history shows adjacent cron delivery and timer work, including preserved isolated message targets, delivery target tracing, and deferred heartbeat target fixes, which may be useful for reviewing the timer/finalization side of this PR. (role: adjacent cron delivery/timer maintainer; confidence: medium; commits: 7d9a9d83ffca, 4c8299ca3d4f, 1d4e4314dddc; files: src/cron/isolated-agent/run-executor.ts, src/cron/service/timer.ts)

Remaining risk / open question:

• The PR intentionally creates a narrow exception to owner-only tool filtering for isolated cron runs. The current diff appears scoped to normalized toolsAllow containing cron and then restricts the cron tool to removing the active job, but this is still an authorization-path change that should receive maintainer/security review.
• The timer change finalizes successful runs after the backing job has been removed. That matches the self-removal use case, but maintainers should confirm the resulting run-history, event, and task-ledger semantics are correct for removed jobs.

Codex review notes: model gpt-5.5, reasoning high; reviewed against fd2625a16252.