🔒 Aisle Security Analysis

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

1. 🟡 Encrypted auth store can be overwritten in plaintext when OPENCLAW_PASSPHRASE is missing

Description

saveSealedJsonFile() will write plaintext JSON whenever OPENCLAW_PASSPHRASE is not set. Combined with the new auth store loading behavior (which swallows SealedJsonPassphraseRequiredError and continues with an empty/coerced store), this enables a downgrade-to-plaintext and data loss scenario:

• If auth-profiles.json (or oauth.json) is sealed and OPENCLAW_PASSPHRASE is missing, loadSealedJsonFile() throws SealedJsonPassphraseRequiredError.
• loadProtectedAuthJson() catches that error and returns undefined, causing the code to proceed as if no store exists.
• Later code paths (e.g., external CLI sync, legacy migration, OAuth merge, or any updater that persists) call saveSealedJsonFile().
• With no passphrase, saveSealedJsonFile() writes the new store in plaintext, overwriting the previously encrypted file (destroying the encrypted data) and potentially persisting newly merged credentials/tokens unencrypted.

Concrete reproduction (one example):

1. Create a sealed auth-profiles.json by running the app once with OPENCLAW_PASSPHRASE set.
2. Unset OPENCLAW_PASSPHRASE.
3. Ensure a write-trigger occurs, e.g.:
   • external CLI credentials are present so syncExternalCliCredentials() returns true, or
   • a legacy auth.json exists (migration), or
   • a plaintext oauth.json exists (merge), or
   • any command that updates the store via saveAuthProfileStore() / upsert...WithLock().
4. Run the app/load path that persists; observe auth-profiles.json becomes plaintext JSON (no sealed prefix).

Vulnerable code:

const plaintext = `${JSON.stringify(data, null, 2)}\n`;
const passphrase = resolvePassphrase(env);
fs.writeFileSync(pathname, passphrase ? sealUtf8(plaintext, passphrase) : plaintext, "utf8");

Recommendation

Prevent silent downgrade/overwrite when an encrypted file exists but the passphrase is missing.

Recommended hardening in saveSealedJsonFile():

• If OPENCLAW_PASSPHRASE is missing and the destination file already exists and is sealed, refuse to write (throw a SealedJsonPassphraseRequiredError).
• Optionally, add a separate explicit “allow plaintext write” flag for non-secret uses.

Example fix:

export function saveSealedJsonFile(pathname: string, data: unknown, env: NodeJS.ProcessEnv = process.env): void {
  const dir = path.dirname(pathname);
  if (!fs.existsSync(dir)) fs.mkdirSync(dir, { recursive: true, mode: 0o700 });

  const passphrase = resolvePassphrase(env);

  if (!passphrase && fs.existsSync(pathname)) {
    const existingRaw = fs.readFileSync(pathname, "utf8");
    if (isSealedJsonText(existingRaw)) {
      ​// Do not overwrite encrypted content with plaintext.
      throw new SealedJsonPassphraseRequiredError(pathname);
    }
  }

  const plaintext = `${JSON.stringify(data, null, 2)}\n`;
  fs.writeFileSync(pathname, passphrase ? sealUtf8(plaintext, passphrase) : plaintext, "utf8");
  fs.chmodSync(pathname, 0o600);
}

Additionally (defense in depth): in auth-profiles/store.ts, when SealedJsonPassphraseRequiredError occurs, consider failing the operation or forcing readOnly/disabling any subsequent persistence for that run.

2. 🟡 Sealed JSON encryption uses implicit scrypt defaults (weak/unstable KDF parameters) and omits KDF metadata

Description

The sealed JSON file format derives the AES-256-GCM key from OPENCLAW_PASSPHRASE using scryptSync() without specifying work factors and without recording KDF parameters in the envelope.

Impacts:

• Offline brute-force risk: if an attacker obtains the sealed file, security depends heavily on passphrase entropy. Node’s default scrypt parameters may be insufficient against GPU/ASIC guessing when operators choose weak env-var passphrases.
• Future decryptability/upgrade risk: because KDF parameters are not stored, any future change in Node’s scrypt defaults (or a desire to tune them) can break decryption compatibility or silently weaken/strengthen encryption across versions.

Vulnerable code:

function deriveKey(passphrase: string, salt: Buffer): Buffer {
  return scryptSync(passphrase, salt, 32);
}

Recommendation

Make KDF parameters explicit and store them in the envelope so encryption strength and decryptability are stable across releases.

Example:

type SealedJsonEnvelopeV1 = {
  v: 1;
  alg: "aes-256-gcm";
  kdf: { name: "scrypt"; N: number; r: number; p: number };
  salt: string;
  iv: string;
  tag: string;
  ciphertext: string;
};

const SCRYPT_PARAMS = { N: 1 << 18, r: 8, p: 1 } as const; // tune for your perf budget

function deriveKey(passphrase: string, salt: Buffer, kdf = SCRYPT_PARAMS): Buffer {
  return scryptSync(passphrase, salt, 32, kdf);
}

​// when sealing:
​// kdf: { name: "scrypt", ...SCRYPT_PARAMS }

​// when unsealing:
​// validate envelope.kdf and pass into deriveKey(...)

Additionally:

• Enforce a minimum passphrase length/entropy (or require a randomly generated key) to reduce real-world brute-force risk.
• Consider allowing operators to configure work factors (while still writing the chosen factors into the envelope).

3. 🟡 Auth store written with insecure permissions window (chmod after write)

Description

saveSealedJsonFile() writes sensitive auth material (API keys / OAuth refresh tokens) to disk using fs.writeFileSync() and only then applies chmod(0o600).

On POSIX systems, newly-created files are typically created with mode 0o666 & ~umask (often 0644). This creates a TOCTOU window where another local user on the same host (who can traverse the directory, e.g. if the state dir is under a 0755 home directory or an overridden shared OPENCLAW_STATE_DIR/OPENCLAW_AGENT_DIR) can open/read the file before permissions are restricted.

Vulnerable code:

fs.writeFileSync(pathname, passphrase ? sealUtf8(plaintext, passphrase) : plaintext, "utf8");
fs.chmodSync(pathname, 0o600);

Recommendation

Write with secure permissions at creation time and do an atomic temp-file + rename update.

You already have an atomic helper (writeTextAtomic) that best-effort applies chmod and uses rename:

import { writeTextAtomic } from "./json-files.js";

export function saveSealedJsonFile(pathname: string, data: unknown, env: NodeJS.ProcessEnv = process.env): void {
  const plaintext = `${JSON.stringify(data, null, 2)}\n`;
  const passphrase = resolvePassphrase(env);
  const payload = passphrase ? sealUtf8(plaintext, passphrase) : plaintext;
  ​// write with mode=0o600 and atomic rename
  ​// (ensureDirMode should be 0o700)
  await writeTextAtomic(pathname, payload, { mode: 0o600, ensureDirMode: 0o700, appendTrailingNewline: false });
}

If you must keep sync APIs, use fs.openSync() with an explicit mode and write to the returned FD (or implement a sync temp+rename path) so there is no world-readable intermediate state.

4. 🟡 Symlink/hardlink TOCTOU when writing sealed JSON files (arbitrary file clobber/chmod)

Description

saveSealedJsonFile() performs non-atomic filesystem operations (existsSync/mkdirSync, writeFileSync, then chmodSync) on a caller-provided pathname without any protection against symlink/hardlink attacks.

If an attacker can influence either:

• the pathname itself (via env overrides such as OPENCLAW_AGENT_DIR, OPENCLAW_STATE_DIR, OPENCLAW_OAUTH_DIR), or
• the directory contents (e.g., they can create/replace the target file with a symlink between the checks and the write),

then they can cause OpenClaw to:

• overwrite an arbitrary file that the OpenClaw process has permission to write (via symlink followed by writeFileSync), and/or
• change permissions of an arbitrary file to 0600 (via symlink followed by chmodSync).

This is particularly risky if OpenClaw is run with elevated privileges or in shared directories, and it is realistic in multi-user systems when the state dir location is overridden to a world-writable location (e.g., /tmp/...).

Vulnerable code:

if (!fs.existsSync(dir)) {
  fs.mkdirSync(dir, { recursive: true, mode: 0o700 });
}
fs.writeFileSync(pathname, payload, "utf8");
fs.chmodSync(pathname, 0o600);

loadSealedJsonFile() similarly uses existsSync + readFileSync without validating that the file is a regular file (not a symlink), enabling an attacker who can swap in a symlink to redirect reads to other files (less severe but still a confused-deputy / integrity concern).

Recommendation

Harden against symlink/hardlink attacks and TOCTOU:

• Prefer an atomic temp-file + rename approach inside the same directory.
• When opening/writing, use fs.open/fs.openSync with flags that avoid following symlinks where available.
  • On Linux, you can use O_NOFOLLOW via fs.constants.O_NOFOLLOW.
• Validate the target is a regular file using lstat/fstat.
• For directory creation, consider validating that each path component is a directory and not a symlink.

Example (conceptual; platform-conditional):

import { constants as c } from "node:fs";

const fd = fs.openSync(pathname, c.O_WRONLY | c.O_CREAT | c.O_TRUNC | (c.O_NOFOLLOW ?? 0), 0o600);
try {
  fs.writeFileSync(fd, payload, "utf8");
  fs.fsyncSync(fd);
} finally {
  fs.closeSync(fd);
}

Even better, reuse the existing atomic writer (writeTextAtomic) but extend it to prevent symlink following (e.g., write to a newly-created temp file with mode 0600 and then rename over an existing file only after verifying it is not a symlink). Also, wrap chmodSync in a try/catch to avoid crashes on platforms where chmod is unsupported.

Analyzed PR: #39059 at commit 9761a71

_{Last updated on: 2026-03-07T18:01:35Z}

Greptile Summary

This PR adds three security hardening measures: bounded GatewayClient request timeouts (30 s default, clamped to [1 ms, 300 s]), optional AES-256-GCM at-rest encryption for auth-profiles.json / oauth.json gated on OPENCLAW_PASSPHRASE, and a best-effort chmod(0o600) re-assertion after mirrored transcript writes.

The gateway timeout and transcript permission changes are well-implemented. The sealed-JSON infrastructure is cryptographically sound, but two correctness issues were found in how the new sealed reader is wired into the auth-store load paths:

• Subagent auth inheritance broken when passphrase is set (store.ts line 412): the subagent fallback path still calls loadJsonFile(mainAuthPath) rather than loadProtectedAuthJson. Because loadJsonFile cannot parse sealed content, subagents silently fail to inherit credentials from the main store whenever OPENCLAW_PASSPHRASE is configured.
• Sealed store silently downgraded to plaintext (store.ts lines 435–438): if OpenClaw starts without OPENCLAW_PASSPHRASE while a sealed store exists, loadCoercedStore returns null and the legacy/OAuth/CLI migration write path triggers with no passphrase, overwriting the sealed file in plaintext — contrary to the PR's "fail-closed" claim.
• The scryptSync call in sealed-json-file.ts uses implicit Node.js defaults (N=16384); explicitly documenting the cost parameters would improve auditability.

Confidence Score: 2/5

• Not safe to merge as-is due to a subagent auth regression and a sealed-store downgrade path introduced by the PR.
• The gateway timeout and transcript permission changes are correct and well-tested. However, the sealed auth-store integration has two logic bugs: subagent credential inheritance silently breaks when OPENCLAW_PASSPHRASE is set (wrong reader still used on line 412), and a write-path can silently convert a sealed store back to plaintext when the passphrase is absent at startup. Both issues undermine the security guarantees the PR is trying to provide.
• src/agents/auth-profiles/store.ts — two separate correctness/security issues in the sealed store read and write paths.

_{Last reviewed commit: 9761a71}