Description

The openclaw flows CLI commands do not enforce owner scoping when resolving flows.

• flows show and flows cancel call resolveFlowForLookupToken(opts.lookup), which accepts either a flowId or an ownerKey and returns the latest flow for that owner.
• flows cancel then calls cancelFlowById({ flowId }) (non-owner-scoped), allowing cancellation of flows belonging to other owners.
• flows list enumerates all flow records and can output full flow metadata plus linked tasks in JSON.

If the CLI is usable by untrusted users in a shared/multi-tenant environment (e.g., multiple agents/owners on the same deployment, remote execution wrappers, exposed admin endpoints), a caller can:

• Read another owner’s flow metadata (including ownerKey, goal, stateJson/waitJson via JSON output, and linked tasks)
• Cancel another owner’s running flow by providing their flowId or ownerKey

Vulnerable code:

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

Recommendation

Enforce owner scoping (or explicit admin authorization) for all flow inspection and mutation operations.

Options:

1. If CLI actions should be owner-scoped: require/derive a callerOwnerKey (from config/session identity) and use the owner-scoped helpers:

import { resolveFlowForLookupTokenForOwner } from "../tasks/flow-owner-access.js";
import { cancelFlowByIdForOwner } from "../tasks/task-executor.js";

const flow = resolveFlowForLookupTokenForOwner({
  token: opts.lookup,
  callerOwnerKey,
});
if (!flow) { /* not found */ }

await cancelFlowByIdForOwner({ cfg: loadConfig(), flowId: flow.flowId, callerOwnerKey });

2. If this is intended as an operator/admin-only tool: remove ownerKey-based lookup (accept only flowId), and/or add an explicit --all-owners/--admin gate to list/show/cancel that is disabled by default.

Also consider filtering flows list output to the caller’s owner by default (and requiring explicit elevation for global listings).

Description

The SQLite-backed flow registry store derives its database path from OPENCLAW_STATE_DIR (via resolveFlowRegistrySqlitePath(process.env)), then performs filesystem operations that follow symlinks and are subject to TOCTOU races:

• mkdirSync(dir, { recursive: true }) and chmodSync(dir, 0o700) are executed on dir without verifying it is a real directory and not a symlink.
• For the database file and its WAL sidecars (registry.sqlite, registry.sqlite-wal, registry.sqlite-shm), the code checks existsSync() and then runs chmodSync() on the path. chmod follows symlinks on most platforms, so a pre-planted symlink can cause permission changes on an attacker-chosen target.
• The store then opens SQLite at pathname (new DatabaseSync(pathname)), which may create/clobber the target file if the path is redirected.

If an attacker can influence OPENCLAW_STATE_DIR for a more-privileged process, or can create/replace files under the resolved state directory (including symlinks), they can potentially:

• Change permissions on arbitrary files/directories accessible to the process (via chmodSync).
• Cause the process to create/modify an attacker-chosen SQLite file path (via DatabaseSync(pathname)), potentially clobbering sensitive files.

Vulnerable code:

mkdirSync(dir, { recursive: true, mode: FLOW_REGISTRY_DIR_MODE });
chmodSync(dir, FLOW_REGISTRY_DIR_MODE);
...
if (existsSync(candidate)) {
  chmodSync(candidate, FLOW_REGISTRY_FILE_MODE);
}

Recommendation

Harden filesystem handling for the state directory and SQLite files:

1. Do not follow symlinks when setting permissions. Use lstatSync() to detect symlinks and refuse them, or open file descriptors with O_NOFOLLOW (where available) and operate on the fd.
2. Avoid TOCTOU (existsSync → chmodSync). Prefer opening the file securely first, then fchmodSync(fd, mode).
3. Validate/lock down OPENCLAW_STATE_DIR for privileged contexts (e.g., require it to be under the invoking user's home directory, or ignore env overrides when running with elevated privileges).

Example approach (POSIX-oriented):

import { mkdirSync, lstatSync, openSync, fchmodSync, closeSync } from "node:fs";
import { constants } from "node:fs";

function safeChmodFileNoSymlink(p: string, mode: number) {
  const st = lstatSync(p);
  if (st.isSymbolicLink()) throw new Error(`Refusing to chmod symlink: ${p}`);
  const fd = openSync(p, constants.O_RDWR | constants.O_NOFOLLOW);
  try { fchmodSync(fd, mode); } finally { closeSync(fd); }
}

Also ensure the flow registry directory is created with correct ownership and permissions and is not a symlink before use.

Description

ensureSingleTaskFlow() creates a new flow record and then attempts to link the task to it. If an exception is thrown after the flow is created (e.g., linkTaskToFlowById() throwing ParentFlowLinkError or any unexpected persistence/runtime error), the catch handler logs and returns the original task without deleting the newly created flow record, leaving an orphaned flow.

Impact:

• Orphaned flows can accumulate in the flow registry, bloating persisted state and making flows list/show leak stale metadata.
• Repeated triggering (e.g., by repeatedly creating eligible tasks under conditions that cause linkTaskToFlowById() to throw) can cause unbounded growth → potential denial of service via disk/memory usage.

Vulnerable code:

const flow = createFlowForTask({ task: params.task, requesterOrigin: params.requesterOrigin });
const linked = linkTaskToFlowById({ taskId: params.task.taskId, flowId: flow.flowId });
...
} catch (error) {
  log.warn("Failed to create one-task flow for detached run", { ... });
  return params.task; // flow not deleted here
}

Recommendation

Track the created flow ID and ensure it is deleted on any failure after creation.

One approach is to use a let flowId: string | undefined and a catch/finally cleanup:

let flowId: string | undefined;
try {
  const flow = createFlowForTask({ task: params.task, requesterOrigin: params.requesterOrigin });
  flowId = flow.flowId;
  const linked = linkTaskToFlowById({ taskId: params.task.taskId, flowId });
  if (!linked || linked.parentFlowId !== flowId) {
    deleteFlowRecordById(flowId);
    return linked ?? params.task;
  }
  return linked;
} catch (e) {
  if (flowId) {
    deleteFlowRecordById(flowId);
  }
  throw e; // or return params.task, but after cleanup
}

If the intention is to swallow errors and continue, still perform the cleanup before returning.

Description

The new openclaw flows CLI commands output full FlowRecord objects and linked TaskRecord objects when --json is used.

Because these records include fields like:

• FlowRecord.ownerKey (session key / owner identifier)
• FlowRecord.requesterOrigin (DeliveryContext containing channel, to, accountId, threadId)
• task-level identifiers such as TaskRecord.requesterSessionKey, TaskRecord.ownerKey, TaskRecord.childSessionKey, TaskRecord.runId, and potentially error strings

…the JSON output can disclose PII (chat/account identifiers), internal routing/session keys, and operational metadata into terminals, CI logs, or shared artifacts if users run these commands in automated environments.

Vulnerable code (JSON output includes raw records):

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

and:

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

While this is a local CLI, the disclosure is still impactful because JSON output is commonly redirected/archived, and these fields are not obviously safe for broad exposure.

Recommendation

Redact or minimize sensitive fields by default, and require an explicit opt-in for verbose/raw dumping.

Suggested approach:

• For --json, return a sanitized DTO that excludes/obfuscates identifiers like ownerKey, requesterOrigin.to, requesterOrigin.accountId, requesterSessionKey, childSessionKey, etc.
• Add a separate flag such as --json-verbose (or --unsafe-json) for full internal records.

Example:

function redactDeliveryContext(ctx?: DeliveryContext) {
  if (!ctx) return undefined;
  return {
    channel: ctx.channel,
    ​// Avoid emitting direct message targets/account ids by default
    to: ctx.to ? "[redacted]" : undefined,
    accountId: ctx.accountId ? "[redacted]" : undefined,
    threadId: ctx.threadId != null ? "[redacted]" : undefined,
  };
}

function toSafeFlow(flow: FlowRecord) {
  return {
    flowId: flow.flowId,
    syncMode: flow.syncMode,
    status: flow.status,
    revision: flow.revision,
    goal: flow.goal,
    controllerId: flow.controllerId,
    requesterOrigin: redactDeliveryContext(flow.requesterOrigin),
    ​// omit ownerKey/stateJson/waitJson unless verbose
  };
}

​// in --json path:
flows: flows.map((flow) => ({
  ...toSafeFlow(flow),
  taskSummary: getFlowTaskSummary(flow.flowId),
})),

Document the security/privacy implications in CLI help text for the verbose option.

Description

The flow registry restore path can be crashed (and repeatedly re-crashed on subsequent CLI runs) by a corrupted or partially-migrated SQLite registry where flow_runs.owner_key is NULL or blank.

• ensureSchema() conditionally adds owner_key via ALTER TABLE without a NOT NULL constraint and only backfills from owner_session_key when present.
• If any existing row lacks owner_session_key (or the DB is attacker-modified), owner_key can remain NULL.
• loadFlowRegistryStateFromSqlite() converts rows into FlowRecord objects without validating owner_key.
• During restore, normalizeRestoredFlowRecord() calls assertFlowOwnerKey(record.ownerKey) which throws when the value is missing/empty, aborting ensureFlowRegistryReady() and causing commands that touch flows to fail.

Vulnerable code (migration/backfill leaves nullable owner_key):

if (!hasFlowRunsColumn(db, "owner_key") && hasFlowRunsColumn(db, "owner_session_key")) {
  db.exec(`ALTER TABLE flow_runs ADD COLUMN owner_key TEXT;`);
  db.exec(`
    UPDATE flow_runs
    SET owner_key = owner_session_key
    WHERE owner_key IS NULL
  `);
}

And restore-time hard failure:

ownerKey: assertFlowOwnerKey(record.ownerKey),

Recommendation

Harden both the migration and restore paths so a corrupted registry cannot brick the CLI.

1. Migration/backfill: ensure all rows end up with a non-null owner_key.

   • If you can safely derive it, backfill it for all rows.
   • Otherwise, delete/quarantine invalid rows.

2. Enforce invariants: SQLite cannot add a NOT NULL constraint via ALTER TABLE ... ADD COLUMN in-place for existing tables; consider rebuilding the table or adding a validation step.

3. Restore robustness: do not let a single bad row crash restore. Skip invalid rows (and optionally log/repair).

Example approach (repair on load):

​// after ALTER TABLE
db.exec(`UPDATE flow_runs SET owner_key = '' WHERE owner_key IS NULL;`);
​// OR delete invalid rows
db.exec(`DELETE FROM flow_runs WHERE owner_key IS NULL OR trim(owner_key) = ''`);

Example approach (defensive restore):

for (const [flowId, flow] of restored.flows) {
  try {
    flows.set(flowId, normalizeRestoredFlowRecord(flow));
  } catch {
    ​// skip or repair
  }
}