🔒 Aisle Security Analysis

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

1. 🟠 Prototype pollution via unvalidated sessionKey used as plain-object key in session store writes

Description

The session store is represented as a plain JavaScript object (Record<string, SessionEntry>) and is indexed and written using sessionKey directly.

Because JavaScript objects treat certain property names specially (notably __proto__), an attacker who can control sessionKey can trigger prototype mutation/pollution when the code assigns store[sessionKey] = ....

Why this is exploitable:

• Input control: Some ingress paths accept a caller-supplied session key (e.g., HTTP header x-openclaw-session-key via resolveGatewayRequestContext() used by openresponses-http.ts, and RPC params like send.sessionKey). These values are not guaranteed to be canonicalized to agent:<id>:... before reaching the session-store mutation helpers.
• Sink: persistSessionEntry() writes store[params.sessionKey] = merged; inside updateSessionStore().
• If params.sessionKey is "__proto__", the assignment can invoke the __proto__ setter on Object.prototype, changing the prototype of the in-memory store object and potentially impacting subsequent logic that relies on property lookups.

Vulnerable code:

const persisted = await updateSessionStore(params.storePath, (store) => {
  const merged = mergeSessionEntry(store[params.sessionKey], params.entry);
  ​// ...
  store[params.sessionKey] = merged;
  return merged;
});

Impact depends on surrounding usage, but prototype mutation can lead to:

• unexpected behavior when reading keys from the store (prototype chain interference)
• denial of service (hard-to-debug crashes/logic errors)
• unsafe interactions with other code that assumes a “normal” object prototype

Recommendation

Harden the session store against unsafe keys. Prefer a null-prototype dictionary (or Map) for all session-store objects, and/or canonicalize/validate session keys before any indexing.

Option A: Use a null-prototype store object

Convert parsed stores to Object.create(null) and ensure all in-memory stores retain a null prototype.

function toNullProtoStore<T>(obj: Record<string, T>): Record<string, T> {
  const out: Record<string, T> = Object.create(null);
  for (const [k, v] of Object.entries(obj)) out[k] = v;
  return out;
}

​// after JSON.parse(raw)
const parsed = JSON.parse(raw);
if (isSessionStoreRecord(parsed)) {
  store = toNullProtoStore(parsed);
}

Option B: Validate/normalize keys at boundaries

Reject keys that can mutate prototypes when used as property names:

const FORBIDDEN_KEYS = new Set(["__proto__", "prototype", "constructor"]);
function assertSafeSessionKey(key: string) {
  const k = key.trim().toLowerCase();
  if (FORBIDDEN_KEYS.has(k)) throw new Error("invalid sessionKey");
}

assertSafeSessionKey(params.sessionKey);

Additionally, consider canonicalizing explicit session keys into the safe agent:<id>:... form before persisting to the store.

2. 🔵 TOCTOU + symlink-following in ACP transcript persistence can overwrite/truncate arbitrary files

Description

The ACP transcript persistence flow performs a time-of-check/time-of-use pattern and then writes to the transcript path without defending against symlinks.

• persistAcpTurnTranscript() checks existence via fs.access(sessionFile) and stores the result in hadSessionFile.
• It then opens the path (SessionManager.open(sessionFile)) and calls prepareSessionManagerForRun().
• prepareSessionManagerForRun() may truncate the transcript path using fs.writeFile(sessionFile, "").

Because the path is opened/written by pathname and no lstat/O_NOFOLLOW/inode verification is performed, an attacker with write access to the transcripts directory (e.g., due to permissive directory permissions or a shared/writable state directory) can replace sessionFile with a symlink between the access check and subsequent writes, causing the process to append to or truncate an arbitrary file accessible to the running user.

Vulnerable code (TOCTOU + unsafe open-by-path):

const hadSessionFile = await fs
  .access(sessionFile)
  .then(() => true)
  .catch(() => false);
const sessionManager = SessionManager.open(sessionFile);
await prepareSessionManagerForRun({ sessionManager, sessionFile, hadSessionFile, ... });

Recommendation

Eliminate the pre-check/open TOCTOU and add symlink-resistant file handling for transcript paths.

Recommended defense-in-depth:

1. Reject symlinks before opening/truncating:

import fs from "node:fs/promises";

async function assertNotSymlink(p: string) {
  const st = await fs.lstat(p).catch(() => null);
  if (st?.isSymbolicLink()) throw new Error(`Refusing to write to symlink: ${p}`);
}

await assertNotSymlink(sessionFile);

2. If you control the low-level open inside transcript persistence, open with O_NOFOLLOW (when available) and use the resulting fd for writes. Node example:

import fs from "node:fs/promises";
import fssync from "node:fs";

const flags = fssync.constants.O_CREAT | fssync.constants.O_APPEND | fssync.constants.O_WRONLY |
  (typeof fssync.constants.O_NOFOLLOW === "number" ? fssync.constants.O_NOFOLLOW : 0);

const fh = await fs.open(sessionFile, flags, 0o600);
try {
  ​// write using fh.write / fh.appendFile
} finally {
  await fh.close();
}

3. Avoid truncating-by-path; if truncation is necessary, do it using an fd opened safely (or write a new file and atomic-rename into place).

4. Ensure transcript directories are created with restrictive permissions (e.g., 0o700) to prevent untrusted users from placing symlinks in the first place.

Analyzed PR: #40137 at commit 62de5d5

_{Last updated on: 2026-03-08T20:00:21Z}

Greptile Summary

This PR fixes a reliability gap where ACP child session transcripts were not persisted after successful runs, causing sessions_history to appear empty. The fix adds transcript persistence at turn-level, records spawnedBy lineage for ACP children, and extracts shared transcript resolution logic. Failures at spawn-time and turn-level are handled as best-effort (logged warnings, non-blocking), so ACP execution is never blocked by storage issues.

Confidence Score: 5/5

• This PR is safe to merge with no identified issues; all failures are handled best-effort and cannot regress existing ACP execution.
• The PR implements transcript persistence for ACP child sessions with comprehensive test coverage and proper error handling. All changes are well-scoped to ACP-specific logic; failures are caught and logged rather than thrown, ensuring the spawn always proceeds. No security, data-loss, or breaking-change risks are present. The implementation correctly normalizes keys and preserves exact transcript whitespace as intended.
• No files require special attention. All changes are sound and thoroughly tested.

_{Last reviewed commit: bb12350}

Merged via squash.

• Prepared head SHA: 62de5d56691e1fd185cc617bb5e7283a14ce90b0
• Merge commit: 404b1527e6bdf84a2b976f80111e5e5334c10e7c

Thanks @mbelinky!