🔒 Aisle Security Analysis

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

1. 🟠 Rescue watchdog onboarding can leak rescue gateway token via weak loopback port ownership verification (TOCTOU/port hijack)

Description

setupRescueWatchdog() provisions a rescue gateway token and later connects to ws://127.0.0.1:<rescuePort> using that token. The intended protection is waitForRescueGatewayIdentity(), but the identity check can be satisfied by a non-OpenClaw process in some environments.

Key issues:

• TOCTOU port allocation race: allocateLoopbackPort() selects an ephemeral port by binding and then closing it; another local process can bind that port before the managed gateway successfully starts.
• Weak “gateway identity” heuristic: waitForRescueGatewayIdentity() treats a WebSocket close code 1008 with an auth-like reason as “looks like a gateway”. A malicious WebSocket server can trivially mimic this.
• Unsafe fallback when ownership metadata is unavailable: if service.readRuntime() returns unknown/no PID and inspectPortUsage() can’t identify the listener process (e.g., missing lsof/insufficient privileges → listeners.length === 0), the function will accept the probe result without confirming the listener belongs to the managed service.
• After this acceptance, onboarding calls waitForGatewayReachable() / callGateway() with the rescue token, disclosing it to whatever process is actually bound to that loopback port.

Impact:

• A local attacker (another user on the same host) can potentially capture the rescue gateway token and then use it to access the rescue gateway API.
• Because the rescue profile config copies auth, secrets, models, etc., token compromise may enable exfiltration of sensitive configuration/secrets via gateway methods.

Vulnerable code:

const ownershipMetadataUnavailable =
  runtime.status !== "stopped" &&
  (runtime.status === "unknown" || runtime.pid == null) &&
  (portUsage.status !== "busy" || portUsage.listeners.length === 0);

const probeLooksLikeGateway =
  probe?.ok === true || looksLikeAuthClose(probe?.close?.code, probe?.close?.reason);

if (probeLooksLikeGateway && (listenerOwnedByRuntime || ownershipMetadataUnavailable)) {
  return;
}

Recommendation

Tighten the port ownership check so the onboarding flow never sends the rescue token unless the listener is strongly attributable to the managed service.

Recommended changes:

1. Remove the unauthenticated “auth-close (1008)” heuristic as an identity signal. Only accept probe.ok === true and only after verifying listener ownership.
2. Require verifiable listener ownership:
   • Require inspectPortUsage() to return at least one listener with a PID.
   • Require that PID to match the supervisor-reported PID (or a child PID) when available.
   • If runtime PID is unavailable, fail closed (ask the user to install lsof/run with needed privileges) rather than proceeding.
3. Consider reducing TOCTOU:
   • Avoid “allocate then release” where possible (e.g., choose a deterministic port and check it’s free right before starting; or retry with a new port if bind fails).

Example safer logic (fail closed when you cannot attribute the port):

const probe = await probeGateway({ url: wsUrl, timeoutMs: 1_000 }).catch(() => null);
if (!probe?.ok) {
  await sleep(POLL);
  continue;
}

if (
  runtime.status !== "running" ||
  typeof runtime.pid !== "number" ||
  portUsage.status !== "busy" ||
  portUsage.listeners.length === 0 ||
  !portUsage.listeners.some(l => l.pid === runtime.pid || l.ppid === runtime.pid)
) {
  throw new Error("Refusing to send rescue token: could not prove managed service owns the port");
}

This ensures the onboarding process does not disclose credentials to an untrusted local listener.

2. 🟡 TOCTOU symlink swap allows LaunchAgent plist write outside intended directory (parent directory not protected)

Description

The new ensureSecureDirectory() and writeLaunchAgentPlistSecure() add some symlink checks and use O_NOFOLLOW for the temporary file, but there is still a time-of-check/time-of-use gap and symlink traversal via parent directories:

• installLaunchAgent() checks/creates the LaunchAgents directory via ensureSecureDirectory(path.dirname(plistPath)).
• Later, writeLaunchAgentPlistSecure() computes dir = path.dirname(plistPath) and opens tempPath with O_NOFOLLOW.
• O_NOFOLLOW only protects the final path component (tempPath) and does not prevent following a symlink in any parent component (e.g., if ~/Library/LaunchAgents is swapped to a symlink after ensureSecureDirectory returns).
• Because writeLaunchAgentPlistSecure() only calls lstat() on plistPath/tempPath (which follows symlinks in parent components during path resolution), it does not detect a symlinked parent directory.

Impact (local attack): if an attacker can rename/replace ~/Library/LaunchAgents (possible when ~/Library is group/world-writable, and this patch also removed previous attempts to tighten home/~/Library), they can race-replace LaunchAgents with a symlink to an attacker-controlled directory. The installer will then write the plist into that other location and proceed to call launchctl bootstrap ... plistPath, potentially enabling code execution as the victim user.

Vulnerable code (symlink protection is not anchored to a directory file descriptor; parent components can be swapped):

await ensureSecureDirectory(path.dirname(plistPath));
...
await writeLaunchAgentPlistSecure(plistPath, plist);

const dir = path.dirname(plistPath);
const tempPath = path.join(dir, `.${path.basename(plistPath)}.${process.pid}.tmp`);
const handle = await fs.open(tempPath, O_NOFOLLOW | ...);

Recommendation

Anchor operations to a verified, non-symlink directory and reduce TOCTOU windows.

Practical hardening options (best to combine):

1. Re-secure parent directories before use (regression fix): reintroduce permission tightening for home and ~/Library (or at least verify they are not group/world-writable) so other users cannot rename/swap LaunchAgents.

2. Re-check the directory immediately before file creation and before bootstrap, and fail closed if it is a symlink or is group/other-writable.

3. If possible in your runtime/Node version, use OS primitives that avoid path re-resolution (preferred): open the target directory with O_DIRECTORY | O_NOFOLLOW, then create/write the file relative to that directory using openat-style APIs (or a native helper) so symlink swapping in parent components cannot redirect the write.

Example of additional directory verification (defense-in-depth):

async function assertSecureDir(dir: string) {
  const st = await fs.lstat(dir);
  if (!st.isDirectory() || st.isSymbolicLink()) throw new Error(`Insecure dir: ${dir}`);
  const mode = st.mode & 0o777;
  if ((mode & 0o022) !== 0) throw new Error(`Dir is writable by group/other: ${dir}`);
}

await assertSecureDir(path.dirname(plistPath));
await writeLaunchAgentPlistSecure(plistPath, plist);
await assertSecureDir(path.dirname(plistPath)); // again before launchctl bootstrap

This does not fully eliminate TOCTOU without openat, but it significantly raises the bar and restores the previous protection against group/world-writable parent directories.

3. 🟡 Symlink-following, non-atomic write of auth-profiles.json can overwrite arbitrary files and risk secret exposure

Description

setupRescueWatchdog() (new rescue onboarding flow) copies API keys/tokens into the rescue profile via saveAuthProfileStore(...). That eventually persists credentials using saveJsonFile(), which performs a plain writeFileSync() to the target path.

Because saveJsonFile():

• does not guard against symlinks (no lstat/O_NOFOLLOW), so a pre-existing auth-profiles.json symlink can redirect the write to an arbitrary file (CWE-59)
• is not atomic (direct truncation+write), so a crash/kill can leave partially-written JSON and potentially break auth loading
• creates the file with default permissions first, then chmod(0o600) after; if the parent directory is accessible to other local users (e.g., state dir overridden to a shared path), there is a brief window where secrets may be written with umask-derived permissions (commonly 0644)

This becomes higher impact with rescue watchdog because it intentionally mirrors credentials into the rescue agent directory.

Vulnerable code:

​// src/infra/json-file.ts
fs.writeFileSync(pathname, `${JSON.stringify(data, null, 2)}\n`, "utf8");
fs.chmodSync(pathname, 0o600);

Call chain in the rescue flow:

• src/commands/onboard-rescue.ts calls saveAuthProfileStore(..., rescueAgentDir)
• src/agents/auth-profiles/store.ts calls saveJsonFile(authPath, payload)
• src/infra/json-file.ts performs the unsafe write

Recommendation

Harden credential/config writes against symlink attacks and make them atomic.

Recommended changes:

1. Write atomically: write to a temp file in the same directory with mode 0o600, fsync, then rename.
2. Refuse symlinks: verify the destination is not a symlink via lstat, and open temp files with O_NOFOLLOW where supported.
3. Set permissions at creation time (not after), to avoid any transient world-readable window.

Example secure implementation (POSIX-focused; best-effort on Windows):

import { constants as fsConstants } from "node:fs";
import fs from "node:fs/promises";
import path from "node:path";

export async function saveJsonFileAtomicNoSymlink(pathname: string, data: unknown) {
  const dir = path.dirname(pathname);
  await fs.mkdir(dir, { recursive: true, mode: 0o700 });

  ​// Refuse writing through an existing symlink
  try {
    const st = await fs.lstat(pathname);
    if (st.isSymbolicLink()) throw new Error("refusing to write to symlink");
  } catch (e: any) {
    if (e?.code !== "ENOENT") throw e;
  }

  const tmp = path.join(dir, `.${path.basename(pathname)}.${process.pid}.tmp`);
  const nofollow = typeof fsConstants.O_NOFOLLOW === "number" ? fsConstants.O_NOFOLLOW : 0;
  const fh = await fs.open(tmp, fsConstants.O_WRONLY | fsConstants.O_CREAT | fsConstants.O_EXCL | nofollow, 0o600);
  try {
    await fh.writeFile(`${JSON.stringify(data, null, 2)}\n`, { encoding: "utf8" });
    await fh.sync();
  } finally {
    await fh.close();
  }
  await fs.rename(tmp, pathname);
}

Then update saveAuthProfileStore() to use this hardened helper for auth-profiles.json writes.

Analyzed PR: #40658 at commit 78a057e

_{Last updated on: 2026-03-12T13:54:27Z}

Greptile Summary

This PR adds opt-in rescue watchdog onboarding — a second isolated gateway profile that monitors and auto-restarts the primary profile. The implementation is well-scoped, correctly isolates the rescue profile env/state from the primary, preserves rescue-only credentials on re-runs, and includes solid unit + integration test coverage.

Key findings:

• Security: Math.random() is used to generate the rescue gateway auth token (resolveRescueGatewayToken). Math.random() is not a CSPRNG; the resulting token has ~53 bits of entropy sourced from a predictable RNG. A cryptographic source (crypto.randomBytes or crypto.randomUUID) should be used instead.
• Robustness: syncRescueAuthProfiles falls back to an empty string when OPENCLAW_STATE_DIR is absent from the rescue env, which silently creates agents/main/agent relative to the process working directory. A guard/assertion would be safer.
• UX: The interactive rescue watchdog confirm prompt fires unconditionally in quickstart flow (whenever no explicit flag is passed), breaking the quickstart contract of minimal user interaction.

Confidence Score: 3/5

• The PR is safe to merge with the token generation security issue addressed first.
• The core logic is sound and tests are thorough, but the use of Math.random() for an auth token that protects a network-accessible gateway service is a meaningful security concern that should be fixed before merging. The other two findings are lower-severity (a defensive coding gap and a minor UX inconsistency in quickstart flow).
• src/commands/onboard-rescue.ts — token generation and OPENCLAW_STATE_DIR fallback; src/wizard/onboarding.finalize.ts — quickstart flow rescue prompt.

_{Last reviewed commit: 813e755}

Addressed the review feedback in follow-up commit e0e57e7f2.

What changed:

• Switched rescue gateway token generation to the existing CSPRNG-backed randomToken() helper instead of Math.random().
• Added an explicit guard for unresolved rescue OPENCLAW_STATE_DIR before touching the rescue auth store path.
• Kept QuickStart minimal by only showing the interactive rescue prompt in non-QuickStart flows unless --rescue-watchdog is passed explicitly.
• Added regression coverage for the QuickStart prompt behavior and kept the setup test covering isolated rescue env/state paths.
• Updated docs to call out the QuickStart behavior.

Local verification after the follow-up:

• pnpm test src/commands/onboard-rescue.test.ts src/commands/onboard-rescue.setup.test.ts src/cli/program/register.onboard.test.ts src/wizard/onboarding.finalize.test.ts
• pnpm check

A new push is up now, so CI should rerun on the updated commits.

Addressed the remaining onboarding/rescue review items in follow-up commit 93b132d78.

What changed:

• Reconcile existing rescue services against the current install plan and reinstall them when the managed command drifts, instead of only restarting an outdated service.
• Gate non-interactive rescue enablement before side effects: rescue profiles no longer force daemon install, and Linux runs without user systemd now skip rescue setup cleanly.
• Added regression coverage for both non-interactive rescue gating and rescue-service reinstall behavior.
• Re-ran detect-secrets on the full PR file set; .secrets.baseline stays clean on the current head.

Local verification on this head:

• pnpm test src/commands/onboard-rescue.test.ts src/commands/onboard-rescue.setup.test.ts src/commands/onboard-non-interactive/local.test.ts src/cli/program/register.onboard.test.ts src/wizard/onboarding.finalize.test.ts
• pnpm check
• /usr/bin/python3 -m pre_commit run detect-secrets --files docs/cli/onboard.md docs/reference/wizard.md docs/start/wizard-cli-automation.md docs/start/wizard-cli-reference.md docs/start/wizard.md src/cli/program/register.onboard.test.ts src/cli/program/register.onboard.ts src/commands/onboard-non-interactive/local.ts src/commands/onboard-non-interactive/local/output.ts src/commands/onboard-rescue.setup.test.ts src/commands/onboard-rescue.test.ts src/commands/onboard-rescue.ts src/commands/onboard-types.ts src/wizard/onboarding.finalize.test.ts src/wizard/onboarding.finalize.ts

Follow-up is in 27f3d63.

This addresses the remaining review items on this PR:

• non-interactive --rescue-watchdog now exits non-zero if setupRescueWatchdog() fails instead of returning success
• interactive onboarding now skips rescue provisioning when the primary managed service install failed earlier in the same run
• rescue setup now rejects invalid monitored profile names before deriving profile/state paths
• rescue setup now builds its service env from a narrow allowlist, so ambient proxy env does not get copied into the rescue service definition

Validation:

• pnpm test src/commands/onboard-rescue.test.ts src/commands/onboard-rescue.setup.test.ts src/commands/onboard-non-interactive/local.test.ts src/commands/onboard-non-interactive/local.run.test.ts src/wizard/onboarding.finalize.test.ts
• pnpm tsgo

Local pnpm check is still blocked here by an existing unrelated type mismatch in ui/vitest.config.ts (duplicate Vitest browser provider types in node_modules), not by these rescue changes.

Follow-up is in 1a2d69e.

This closes out the remaining onboarding/CI issues on the current head:

• non-interactive rescue setup now aborts before setupRescueWatchdog() if the primary managed service install failed
• installGatewayDaemonNonInteractive() now returns success/failure so automation can gate rescue provisioning correctly
• macOS launchd install paths now tighten directory/file modes and normalize LaunchAgents/Trash paths with POSIX joins, matching the current main expectations that were failing in Windows merge CI
• detect-secrets now excludes Sparkle sparkle:edSignature appcast metadata so the secrets job no longer flags release signatures as credentials

Validation on this head:

• pnpm test src/daemon/launchd.test.ts src/commands/onboard-non-interactive/local/daemon-install.test.ts src/commands/onboard-non-interactive/local.run.test.ts src/commands/onboard-non-interactive/local.test.ts src/wizard/onboarding.finalize.test.ts
• pnpm tsgo
• /usr/bin/python3 -m pre_commit run detect-secrets --files .detect-secrets.cfg .pre-commit-config.yaml .secrets.baseline appcast.xml

Follow-up is in 1807d4e.

This fixes the remaining long-profile collision in rescue onboarding:

• short monitored profile names still keep the existing <profile>-rescue mapping
• when a monitored profile must be truncated, the derived rescue profile now appends a stable SHA-256 suffix before -rescue, so different valid profiles that share the same long prefix no longer overwrite each other's rescue config/state
• added regression coverage for two long valid profile names with the same 57-character prefix to confirm they resolve to distinct rescue profiles and stay within the profile-name limit

Validation on this head:

• pnpm test src/commands/onboard-rescue.test.ts src/commands/onboard-rescue.setup.test.ts
• pnpm tsgo

Follow-up is in 22cc27d.

This fixes the two remaining rescue onboarding regressions on the current head:

• rescue auth sync now preserves rescue-only credentials while letting the primary profile override stale duplicate profile IDs on onboarding reruns, so rotated primary keys propagate into the rescue store
• rescue config now carries config.env into fresh rescue profiles and keeps rescue-only env keys on reruns while letting the primary profile refresh overlapping env values
• added regression coverage for both the stale-auth rerun case and env inheritance/merge behavior

Validation on this head:

• pnpm test src/commands/onboard-rescue.test.ts src/commands/onboard-rescue.setup.test.ts
• pnpm tsgo

Follow-up is in b1a4d4249.

This tightens the remaining rescue/launchd hardening on the current head:

• rescue onboarding now refuses to overwrite an existing rescue / <profile>-rescue profile unless that profile is already marked as an OpenClaw-managed rescue watchdog for the same monitored profile
• rescue setup allocates an ephemeral loopback port for fresh rescue profiles, preserves an existing managed rescue port on reruns, and proves listener ownership before sending the rescue gateway token
• rescue watchdog state now carries an explicit wizard.rescueWatchdog ownership marker plus a dedicated least-privilege rescue-watchdog agent id for cron runs
• rescue cron jobs now pin agentId, keep allowUnsafeExternalContent: false, and use the dedicated watchdog agent instead of the default agent
• launchd install now rejects symlinked LaunchAgents paths and writes plist files via O_NOFOLLOW + atomic rename
• config schema metadata/validation now includes the new wizard.rescueWatchdog marker fields

Validation on this head:

• pnpm test src/commands/onboard-rescue.test.ts src/commands/onboard-rescue.setup.test.ts src/daemon/launchd.test.ts src/config/schema.help.quality.test.ts
• pnpm tsgo
• pnpm check
• pnpm build

Follow-up in 05cd77e:

• Replaced the rescue watchdog cron payload with a deterministic rescueWatchdog runner, so recovery no longer goes through agentTurn prompt execution.
• Tightened the rescue agent sandbox to allow: [] + deny: ["*"]; the cron job now runs fixed probe/restart/doctor logic and only invokes exact argv for openclaw --profile <profile> doctor --repair --non-interactive.
• Added cron/schema/store normalization for the new payload kind and covered the deterministic runner with unit tests.

Local verification:

• pnpm test src/commands/onboard-rescue.test.ts src/commands/onboard-rescue.setup.test.ts src/cron/normalize.test.ts src/cron/service.jobs.test.ts src/cron/rescue-watchdog.test.ts
• pnpm build
• pnpm lint

pnpm check still stops in the repo's existing ui/vitest.config.ts Vitest type mismatch during pnpm tsgo; this change does not introduce a new local type-check failure outside that pre-existing blocker.

Follow-up in 413b857:

• Fail closed in non-interactive onboarding when OPENCLAW_PROFILE is invalid, before rescue planning can force daemon install.
• Use the process homedir for macOS LaunchAgent plist/trash paths instead of trusting HOME overrides during service management.
• Tighten LaunchAgent plist permissions from 0644 to 0600 and stop chmod-tightening the parent ~/Library path.
• Added regression coverage for invalid rescue profile rejection and conflicting HOME overrides in launchd install.

Local verification on this head:

• pnpm test src/daemon/launchd.test.ts src/commands/onboard-non-interactive/local.test.ts src/commands/onboard-non-interactive/local.run.test.ts
• pnpm lint
• pnpm build
• pnpm check

Follow-up in e8c8286:

• The deterministic rescue watchdog now caps doctor --repair --non-interactive to the cron job's remaining timeout budget instead of a fixed 15s+ floor.
• If restart probing already consumed the configured budget, the watchdog now skips spawning doctor entirely and returns an error, preventing overlapping background repair processes.
• Added a regression test covering the exhausted-budget path.

Local verification:

• pnpm test src/cron/rescue-watchdog.test.ts
• pnpm lint

Follow-up pushed in 1ebacdb.

What changed:

• Stabilized caps per-probe timeout to the remaining probe budget in src/cron/rescue-watchdog.test.ts by removing the brittle mock.calls[1] assumption.
• The test now forces a bounded post-restart probe path and asserts against observed probe timeout values without assuming a fixed call index.

Validation:

• pnpm test src/cron/rescue-watchdog.test.ts ✅
• pnpm build ✅
• pnpm check ✅
• pnpm test ⚠️ fails in this sandbox due unrelated environment constraints (listen EPERM for local socket-binding tests and transient DNS ENOTFOUND in remote-provider tests), not due this test change.

All review threads have been resolved and CI is green. This PR adds an opt-in rescue watchdog for self-healing gateway recovery during onboarding.

Would appreciate a review when you have a moment — cc @joshavant @tyler6204

Commit history cleanup: squashed 44 incremental commits into 5 logical commits:

1. feat(rescue): add rescue watchdog core types and shared utilities
2. feat(daemon): add rescue profile service management for launchd, systemd, and schtasks
3. feat(cron): add rescue watchdog job engine
4. feat(onboard): add opt-in rescue watchdog onboarding flow
5. docs(rescue): add rescue watchdog documentation and stabilize test mocks

Also updated the PR description to fill in all required template sections (Human Verification, Compatibility, Failure Recovery, Risks). This is the final force push for history cleanup — incremental commits only going forward.

Ready for review.

Addressed two remaining review items in 78a057e:

• explicit --rescue-watchdog now fails onboarding when primary managed-service install fails (instead of soft-skipping).
• loadExistingRescueConfig now only treats ENOENT as missing; non-ENOENT access errors fail closed.

Also added regression tests in:

• src/wizard/onboarding.finalize.test.ts
• src/commands/onboard-rescue.setup.test.ts

Validation in this sandbox is blocked by dependency install DNS failures (ENOTFOUND registry.npmjs.org), so full pnpm build && pnpm check && pnpm test could not complete.