🔒 Aisle Security Analysis

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

1. 🟠 SSH MITM risk: Remote gateway probe auto-trusts unknown host keys (StrictHostKeyChecking=accept-new)

Description

RemoteGatewayProbe.sshCheckCommand performs an SSH reachability check using permissive host key verification:

• The SSH option StrictHostKeyChecking=accept-new automatically trusts the first-seen host key without user confirmation.
• If an attacker can intercept/redirect the initial connection (or if known_hosts is cleared), the app may silently trust the attacker’s SSH server (TOFU without prompt), enabling man-in-the-middle.
• This probe is used as a preflight before establishing remote transport; successful MITM can allow interception/modification of subsequent tunneled traffic and authentication material (e.g., gateway token/device pairing flows) that rely on the SSH channel being to the intended host.

Vulnerable code:

let options = [
    "-o", "BatchMode=yes",
    "-o", "ConnectTimeout=5",
    "-o", "StrictHostKeyChecking=accept-new",
    "-o", "UpdateHostKeys=yes",
]

Recommendation

Do not automatically trust unknown SSH host keys.

Recommended approaches:

1. Fail closed for unknown keys and require the user to explicitly verify/accept the host fingerprint (out-of-band) before continuing.
2. Use an app-managed known_hosts file to avoid relying on global state and to support explicit key pinning.

Example (fail closed + app known_hosts):

let knownHostsPath = "\(appSupportDir)/known_hosts"
let options = [
  "-o", "BatchMode=yes",
  "-o", "ConnectTimeout=5",
  "-o", "StrictHostKeyChecking=yes",
  "-o", "UserKnownHostsFile=\(knownHostsPath)",
  ​// consider disabling automatic rewrite unless you have a UX for it:
  "-o", "UpdateHostKeys=no",
]

For first-time connections, implement a UX that:

• retrieves the presented host key (or fingerprint),
• shows it to the user,
• instructs the user to compare it with the gateway host’s displayed fingerprint,
• and only then writes it to the app’s known_hosts file.

2. 🟡 Remote gateway auth token can be persisted in plaintext config during remote probe flow

Description

The new remote onboarding probe flow syncs gateway configuration to disk before probing connectivity, which can cause the gateway token to be written to a local JSON config file in plaintext.

Key points:

• RemoteGatewayProbe.run() now calls AppStateStore.shared.syncGatewayConfigNow() before performing the probe.
• syncGatewayConfigNow() writes the merged gateway config via OpenClawConfigFile.saveDict(...).
• When the user has typed a token (which sets remoteTokenDirty = true), the token is serialized into the config dictionary as a normal string under gateway.remote.token.
• The config file is stored at OpenClawPaths.configURL (default ~/.openclaw/openclaw.json, or OPENCLAW_CONFIG_PATH override) and saveDict does not enforce restrictive permissions (e.g., 0600) or use Keychain-backed secret storage.
• In onboarding, the probe temporarily switches connectionMode to .remote to “reuse the shared remote endpoint stack”; because syncing only writes remote settings when connectionMode == .remote, this temporary switch is enough to persist remote settings/tokens even if the user did not intend to commit them.

Vulnerable flow (trigger):

@​MainActor
static func run() async -> RemoteGatewayProbeResult {
    AppStateStore.shared.syncGatewayConfigNow()
    ...
}

Persistence sink:

@​MainActor
func syncGatewayConfigNow() {
    let synced = Self.syncedGatewayRoot(... remoteToken: self.remoteToken,
                                        remoteTokenDirty: self.remoteTokenDirty)
    ...
    OpenClawConfigFile.saveDict(synced.root)
}

Impact:

• A gateway auth token is a reusable credential. Persisting it in plaintext on disk can lead to credential theft by local attackers/processes that can read the config file (especially if default filesystem permissions allow broader reads than intended).

Recommendation

Avoid persisting the gateway token in plaintext and avoid writing it during a probe unless the user explicitly confirms saving.

Suggested fixes:

1. Store the token in macOS Keychain and keep only an opaque reference in openclaw.json (e.g., a $secretRef-style identifier).
2. Enforce filesystem permissions for any on-disk config that may contain secrets:
   • ~/.openclaw directory: 0700
   • openclaw.json: 0600

Example (permission hardening after write):

try data.write(to: url, options: [.atomic])
try FileManager.default.setAttributes([.posixPermissions: 0o600], ofItemAtPath: url.path)
try FileManager.default.setAttributes([.posixPermissions: 0o700],
                                     ofItemAtPath: url.deletingLastPathComponent().path)

3. For onboarding probes, keep probe-only settings in memory (do not call syncGatewayConfigNow()), or gate persistence behind a dedicated “Save remote settings” / “Remember token” toggle.

3. 🔵 Untrusted gateway-provided auth error message logged with OSLog privacy .public

Description

The gateway's websocket connect response can include an arbitrary error message string. This string is propagated into GatewayConnectAuthError.message/errorDescription and then logged using privacy: .public.

• Input (remote-controlled): res.error?["message"] from the gateway JSON response
• Propagation: stored in GatewayConnectAuthError.message and returned via error.localizedDescription
• Sink: logger.error(... \(error.localizedDescription, privacy: .public))

Impact:

• If a gateway endpoint is malicious/compromised (or if a legitimate gateway includes sensitive details in the message), the client will write that data into system logs without redaction.
• The message may also contain newlines/control characters, enabling log forging/injection (CWE-117) and making log analysis unreliable.

Vulnerable code (sink):

self.logger.error(
  "gateway watchdog reconnect paused for non-recoverable auth failure \(error.localizedDescription, privacy: .public)"
)

Source of the logged string:

let msg = (res.error?["message"]?.value as? String) ?? "gateway connect failed"
throw GatewayConnectAuthError(message: msg, ...)

Recommendation

Avoid logging remote/server-provided messages as .public.

Recommended mitigations:

1. Log structured, non-user-controlled fields (e.g., detail code / next step) and redact free-form messages:

if let authError = error as? GatewayConnectAuthError {
    self.logger.error(
        "gateway reconnect paused for auth failure code=\(authError.detailCodeRaw ?? "unknown", privacy: .public)"
    )
} else {
    self.logger.error("gateway reconnect paused for auth failure")
}

2. If you must include the message for debugging, mark it private and sanitize control characters (e.g., replace newlines):

let safeMsg = error.localizedDescription
    .replacingOccurrences(of: "\n", with: " ")
    .replacingOccurrences(of: "\r", with: " ")

self.logger.error("gateway auth failure \(safeMsg, privacy: .private)")

Also consider enforcing a max length for server-provided messages before logging/surfacing them.

Analyzed PR: #43100 at commit 00e2ad8

_{Last updated on: 2026-03-11T12:57:01Z}

Greptile Summary

This PR adds structured remote-gateway auth error handling and contextual token-guidance UI to the macOS onboarding and Settings flows. The core change is a new RemoteGatewayProbe type that centralises the connection check (previously duplicated in GeneralSettings) and surfaces typed GatewayConnectAuthError failures rather than a generic string. GatewayConnectAuthError is promoted from a private type to a public API in GatewayErrors.swift with full GatewayConnectAuthDetailCode/GatewayConnectRecoveryNextStep enums, and the wrap() passthrough fix ensures structured auth errors are not swallowed by generic NSError wrapping. The onboarding token field now appears conditionally (on probe result or when a token is already set) instead of always. Test coverage for the new mapping and UI-visibility logic is solid.

• syncGatewayConfigNow() is missing an @MainActor annotation — it accesses multiple main-actor-isolated UI-state properties; all callers today happen to be @MainActor, but there is no compile-time enforcement of this requirement.
• probeRemoteConnection() unconditionally sets connectionMode = .remote before running the probe — if the remote connection section is visible because showAdvancedConnection == true (not because the user selected remote mode), clicking "Check connection" silently commits and persists the remote mode choice, which may be unexpected.

Confidence Score: 4/5

• Safe to merge with minor actor-annotation and UX side-effect concerns noted above.
• The changes are well-scoped, well-tested, and the critical wrap() passthrough fix is necessary for the feature to work correctly. The two flagged issues are a missing @MainActor annotation (no runtime risk today but a future thread-safety hazard) and an implicit connection-mode state change in probeRemoteConnection() (a UX concern rather than a correctness bug). Neither blocks shipping.
• apps/macos/Sources/OpenClaw/AppState.swift (missing @MainActor on syncGatewayConfigNow) and apps/macos/Sources/OpenClaw/OnboardingView+Pages.swift (implicit connectionMode = .remote side effect in probeRemoteConnection).

_{Last reviewed commit: 1ac86d7}

Addressed the follow-up review feedback in 5bb4ed579:

• added an explicit @MainActor annotation to AppState.syncGatewayConfigNow() so the isolation requirement is spelled out at the call site, even though AppState is already main-actor isolated.
• made onboarding's remote probe preserve the user's original connectionMode after probing, while suppressing the feedback reset that would otherwise clear the result on restore.
• added a focused onboarding test for the transient restore path.

Re-ran:

• swift test --package-path apps/macos --filter OnboardingRemoteAuthPromptTests
• swift test --package-path apps/macos --filter GatewayChannelConnectTests

Landed after rebasing the PR head onto main.

• Local gate:
  • pnpm build
  • pnpm check
  • pnpm test failed in unrelated baseline tests: src/memory/embeddings.test.ts and src/memory/embeddings-voyage.test.ts with getaddrinfo ENOTFOUND example.com; reproduced on origin/main
• Prepared head: 00e2ad8
• Merge commit: 144c1b8

Thanks @ngutman!