1. src/tasks/task-executor.ts, line 3224-3232 (link)

   Stale terminal-status check after task cancellations

   isTerminalFlowStatus(flow.status) uses the snapshot captured at the start of the function, before the active-task cancellation loop. For task_mirrored flows, each cancelTaskById call triggers updateTask → syncFlowFromTask, which may already advance the flow into a terminal state. After the loop, refreshedFlow is re-read with the current revision, so the CAS succeeds and writes "cancelled" a second time — resulting in a redundant write and a bumped revision. No data integrity issue (the CAS handles the race), but moving the terminal check to use refreshedFlow would be cleaner.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: src/tasks/task-executor.ts
   Line: 3224-3232
   
   Comment:
   **Stale terminal-status check after task cancellations**
   
   `isTerminalFlowStatus(flow.status)` uses the snapshot captured at the start of the function, before the active-task cancellation loop. For `task_mirrored` flows, each `cancelTaskById` call triggers `updateTask → syncFlowFromTask`, which may already advance the flow into a terminal state. After the loop, `refreshedFlow` is re-read with the current revision, so the CAS succeeds and writes "cancelled" a second time — resulting in a redundant write and a bumped revision. No data integrity issue (the CAS handles the race), but moving the terminal check to use `refreshedFlow` would be cleaner.
   
   How can I resolve this? If you propose a fix, please make it concise.

2. src/tasks/task-executor.ts, line 3157-3163 (link)

   retryBlockedFlowAsQueuedTaskRun does not update flow status for managed flows

   When called on a managed flow, retryBlockedFlowTask creates a new task and links it via parentFlowId, but syncFlowFromTask is a no-op for managed flows. After the call returns retried: true, the flow record still shows status: "blocked". Consider documenting this or explicitly advancing the managed flow status inside retryBlockedFlowTask.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: src/tasks/task-executor.ts
   Line: 3157-3163
   
   Comment:
   **`retryBlockedFlowAsQueuedTaskRun` does not update flow status for managed flows**
   
   When called on a `managed` flow, `retryBlockedFlowTask` creates a new task and links it via `parentFlowId`, but `syncFlowFromTask` is a no-op for managed flows. After the call returns `retried: true`, the flow record still shows `status: "blocked"`. Consider documenting this or explicitly advancing the managed flow status inside `retryBlockedFlowTask`.
   
   How can I resolve this? If you propose a fix, please make it concise.

3. src/tasks/flow-registry.ts, line 2135-2145 (link)

   normalizeJsonBlob called twice per JSON field in buildFlowRecord

   Both the guard check and the spread value call normalizeJsonBlob(params.stateJson) separately, triggering two structuredClone calls per non-undefined JSON field. Capture the result once to avoid double cloning.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: src/tasks/flow-registry.ts
   Line: 2135-2145
   
   Comment:
   **`normalizeJsonBlob` called twice per JSON field in `buildFlowRecord`**
   
   Both the guard check and the spread value call `normalizeJsonBlob(params.stateJson)` separately, triggering two `structuredClone` calls per non-undefined JSON field. Capture the result once to avoid double cloning.
   
   How can I resolve this? If you propose a fix, please make it concise.

Description

The flow registry SQLite store attempts to harden file permissions by calling chmodSync on paths derived from the environment-controlled state directory.

• Input/control surface: resolveFlowRegistryDir() / resolveFlowRegistrySqlitePath() ultimately depend on OPENCLAW_STATE_DIR (via resolveStateDir()), which can be set via environment.
• Dangerous operations: ensureFlowRegistryPermissions() uses mkdirSync(..., {recursive:true}) and then chmodSync() on the directory and on each sidecar file (registry.sqlite, registry.sqlite-wal, registry.sqlite-shm).
• Symlink + TOCTOU issue: it checks existsSync(candidate) and then separately calls chmodSync(candidate). An attacker who can write within the chosen state directory (or influence it to point somewhere writable) could replace candidate with a symlink between the check and the chmod, causing chmodSync to act on an unexpected target (chmod follows symlinks). This can result in unintended permission changes to arbitrary files accessible to the running user, and becomes a privilege escalation risk if the process is ever run with elevated privileges (e.g., as a service or via sudo) while using an attacker-influenced OPENCLAW_STATE_DIR.

Vulnerable code:

for (const suffix of FLOW_REGISTRY_SIDECAR_SUFFIXES) {
  const candidate = `${pathname}${suffix}`;
  if (!existsSync(candidate)) {
    continue;
  }
  chmodSync(candidate, FLOW_REGISTRY_FILE_MODE);
}

Recommendation

Avoid following symlinks and remove the TOCTOU window.

Recommended mitigations:

1. Validate the resolved paths stay within the expected state directory using realpathSync() (or fs.promises.realpath) for both the directory and each candidate, and ensure they have the expected prefix.
2. Refuse to chmod symlinks by using lstatSync() and checking isSymbolicLink() and isFile().
3. Prefer operating on already-open file descriptors (where possible) to avoid races.

Example hardening (best-effort in Node):

import { chmodSync, lstatSync, mkdirSync, realpathSync } from "node:fs";
import path from "node:path";

function safeChmodFile(candidate: string, rootDir: string, mode: number) {
  const resolvedRoot = realpathSync(rootDir);
  const resolvedCandidate = realpathSync(candidate);
  if (!resolvedCandidate.startsWith(resolvedRoot + path.sep)) {
    throw new Error("Refusing to chmod path outside state dir");
  }
  const st = lstatSync(candidate);
  if (st.isSymbolicLink() || !st.isFile()) {
    throw new Error("Refusing to chmod non-regular file");
  }
  chmodSync(candidate, mode);
}

Also consider treating OPENCLAW_STATE_DIR as trusted configuration only (do not allow untrusted callers to influence it) and documenting that the process must not be run with elevated privileges when using user-controlled state directories.

Description

The CLI commands flowsListCommand and flowsShowCommand support --json output that serializes and prints entire flow records and linked task records directly to stdout.

This is risky because these structures can contain sensitive data and identifiers, including:

• FlowRecord.requesterOrigin (delivery context such as channel/accountId/threadId) which may be PII
• FlowRecord.stateJson / FlowRecord.waitJson which are generic JSON blobs and may carry credentials, tokens, message contents, or other secrets depending on workflow/controller implementation
• Task records returned by listTasksForFlowId(...) which include requesterSessionKey, ownerKey, runId, error, etc.

In environments where CLI output is captured (CI logs, support bundles, telemetry, shell history, redirected output), this can lead to unintended disclosure.

Vulnerable code:

flows: flows.map((flow) => ({
  ...flow,
  tasks: listTasksForFlowId(flow.flowId),
  taskSummary: getFlowTaskSummary(flow.flowId),
})),

And:

JSON.stringify({ ...flow, tasks, taskSummary }, null, 2)

Recommendation

Redact or minimize sensitive fields in JSON output by default.

Options (can be combined):

1. Default-safe JSON schema: only emit non-sensitive metadata (ids, status, timestamps, counts), and exclude stateJson, waitJson, requesterOrigin, and session keys unless explicitly requested.
2. Add an explicit --include-private / --unsafe flag to include full records.
3. If full JSON must be supported, implement a redaction function that scrubs common secret keys (e.g., token, authorization, apiKey, secret, password) recursively.

Example (default-safe):

function toPublicFlow(flow: FlowRecord) {
  return {
    flowId: flow.flowId,
    syncMode: flow.syncMode,
    status: flow.status,
    revision: flow.revision,
    controllerId: flow.controllerId ?? null,
    goal: flow.goal,
    currentStep: flow.currentStep ?? null,
    blockedTaskId: flow.blockedTaskId ?? null,
    blockedSummary: flow.blockedSummary ?? null,
    cancelRequestedAt: flow.cancelRequestedAt ?? null,
    createdAt: flow.createdAt,
    updatedAt: flow.updatedAt,
    endedAt: flow.endedAt ?? null,
  };
}

​// usage
flows: flows.map((flow) => ({
  ...toPublicFlow(flow),
  taskSummary: getFlowTaskSummary(flow.flowId),
})),

If task details are required, similarly emit a minimal task projection (taskId/status/label) rather than full task records.

Description

The openclaw flows CLI commands print externally-influenced strings directly to the terminal via runtime.log().

An attacker who can influence task metadata (e.g., task.label, task.task) can inject ANSI escape sequences/control characters into flow.goal (derived from task label/task) and into per-task output lines. This can:

• Spoof or hide terminal output (e.g., overwrite lines, clear screen)
• Trick operators into executing malicious commands by rendering fake prompts/commands
• Corrupt logs (control characters, CRLF)

Although the repo contains sanitizeTerminalText() (which strips ANSI and control characters), it is not used here.

Vulnerable code:

lines.push([
  shortToken(flow.flowId).padEnd(ID_PAD),
  ...,
  truncate(flow.controllerId ?? "n/a", CTRL_PAD).padEnd(CTRL_PAD),
  counts.padEnd(14),
  truncate(flow.goal, 80),
].join(" "));
...
runtime.log(`goal: ${flow.goal}`);
...
runtime.log(`- ${task.taskId} ${task.status} ${task.runId ?? "n/a"} ${task.label ?? task.task}`);

Recommendation

Sanitize any untrusted text before rendering to the terminal.

Use the existing helper sanitizeTerminalText() (strips ANSI + control chars) for user/external fields such as flow.goal, flow.controllerId, task.label, and task.task.

Example fix:

import { sanitizeTerminalText } from "../terminal/safe-text.js";

const safeGoal = sanitizeTerminalText(flow.goal);
const safeController = sanitizeTerminalText(flow.controllerId ?? "n/a");

runtime.log(`goal: ${safeGoal}`);
...
runtime.log(
  `- ${sanitizeTerminalText(task.taskId)} ${task.status} ${task.runId ?? "n/a"} ` +
  `${sanitizeTerminalText(task.label ?? task.task)}`,
);

Optionally, update truncate() (or create truncateSafe()) to sanitize before truncation so all table cells are safe by default.

Description

The flow cancellation API has an owner-scoped variant (cancelFlowByIdForOwner) but the unscoped cancelFlowById remains exported and is used directly by the CLI flowsCancelCommand.

• cancelFlowById cancels tasks and marks the flow cancelled solely based on a provided flowId.
• No verification is performed that the caller is authorized for the flow’s ownerKey.
• flowsCancelCommand resolves the target flow via resolveFlowForLookupToken, which accepts either a flow id or an owner key (returns latest flow for that owner). This allows cancelling another owner’s flow if an attacker can supply/guess a victim flowId or ownerKey and can reach this command surface.

Vulnerable code:

export async function cancelFlowById(params: { cfg: OpenClawConfig; flowId: string }) {
  const flow = getFlowById(params.flowId);
  ​// ... cancels tasks and updates flow status without owner check
}

const flow = resolveFlowForLookupToken(opts.lookup);
const result = await cancelFlowById({ cfg: loadConfig(), flowId: flow.flowId });

Recommendation

Enforce owner scoping at the call boundary(s) that can be influenced by untrusted input.

Option A (preferred): require callerOwnerKey for cancellation and remove/limit the unscoped export:

export async function cancelFlowByIdForOwner(params: {
  cfg: OpenClawConfig;
  flowId: string;
  callerOwnerKey: string;
}) {
  const flow = getFlowByIdForOwner({ flowId: params.flowId, callerOwnerKey: params.callerOwnerKey });
  if (!flow) return { found: false, cancelled: false, reason: "Flow not found." };
  return cancelFlowById({ cfg: params.cfg, flowId: flow.flowId });
}

Then update flowsCancelCommand (and any other external surface) to use an owner-scoped resolver and cancellation:

const flow = resolveFlowForLookupTokenForOwner({ token: opts.lookup, callerOwnerKey });
const result = await cancelFlowByIdForOwner({ cfg: loadConfig(), flowId: flow.flowId, callerOwnerKey });

Option B: if unscoped cancellation is required for internal/admin use, make it non-exported or clearly mark it as privileged and ensure only admin-only paths can call it.

Description

noteWorkspaceStatus() now emits TaskFlow recovery hints via note() that include flow.flowId and flow.blockedTaskId directly in the note body.

If an attacker can influence these identifiers (or other linked metadata persisted in the flow registry), they can embed control characters / ANSI escape sequences, resulting in terminal escape injection within the interactive doctor output.

note() currently wraps text but does not sanitize ANSI/control characters, and @​clack/prompts notes render directly to terminal.

Vulnerable code:

findings.push(
  `${flow.flowId}: blocked flow points at missing task ${flow.blockedTaskId}; inspect before retrying.`,
);
...
note([...suspicious.slice(0, 5), ...].join("\n"), "TaskFlow recovery");

Recommendation

Sanitize any flow/task identifiers before embedding them in terminal notes.

Example:

import { sanitizeTerminalText } from "../terminal/safe-text.js";

findings.push(
  `${sanitizeTerminalText(flow.flowId)}: blocked flow points at missing task ` +
  `${sanitizeTerminalText(flow.blockedTaskId)}; inspect before retrying.`,
);

Alternatively, enforce sanitization inside note()/wrapNoteMessage() so all note content is safe by default (but ensure you don't break intentional formatting for trusted strings).