From 49d0def6d1e88f002026b1d2a35aa615d48a751a Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Thu, 19 Feb 2026 11:07:56 +0100 Subject: [PATCH] fix(security): harden imessage remote scp/ssh handling --- CHANGELOG.md | 1 + docs/channels/imessage.md | 5 ++ docs/gateway/configuration-reference.md | 3 +- docs/platforms/mac/remote.md | 1 + src/auto-reply/reply/stage-sandbox-media.ts | 14 +++-- src/config/config.schema-regressions.test.ts | 27 +++++++++ src/config/types.imessage.ts | 2 +- src/config/zod-schema.providers-core.ts | 6 +- src/imessage/monitor/monitor-provider.ts | 20 +++++-- src/infra/scp-host.test.ts | 19 ++++++ src/infra/scp-host.ts | 62 ++++++++++++++++++++ src/infra/ssh-tunnel.ts | 2 +- 12 files changed, 150 insertions(+), 12 deletions(-) create mode 100644 src/infra/scp-host.test.ts create mode 100644 src/infra/scp-host.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index da2ae55c91..3fd8b51a5b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ Docs: https://docs.openclaw.ai - Gateway/WebChat: block `sessions.patch` and `sessions.delete` for WebChat clients so session-store mutations stay restricted to non-WebChat operator flows. Thanks @allsmog for reporting. - Security/Skills: for the next npm release, reject symlinks during skill packaging to prevent external file inclusion in distributed `.skill` archives. Thanks @aether-ai-agent for reporting. +- Security/iMessage: harden remote attachment SSH/SCP handling by requiring strict host-key verification, validating `channels.imessage.remoteHost` as `host`/`user@host`, and rejecting unsafe host tokens from config or auto-detection. Thanks @allsmog for reporting. - Security/Feishu: prevent path traversal in Feishu inbound media temp-file writes by replacing key-derived temp filenames with UUID-based names. Thanks @allsmog for reporting. - LINE/Security: harden inbound media temp-file naming by using UUID-based temp paths for downloaded media instead of external message IDs. (#20792) Thanks @mbelinky. - Security/Refactor: centralize hardened temp-file path generation for Feishu and LINE media downloads via shared `buildRandomTempFilePath` helper to reduce drift risk. (#20810) Thanks @mbelinky. diff --git a/docs/channels/imessage.md b/docs/channels/imessage.md index 2876be3137..4adffd7f41 100644 --- a/docs/channels/imessage.md +++ b/docs/channels/imessage.md @@ -103,6 +103,8 @@ exec ssh -T gateway-host imsg "$@" ``` If `remoteHost` is not set, OpenClaw attempts to auto-detect it by parsing the SSH wrapper script. + `remoteHost` must be `host` or `user@host` (no spaces or SSH options). + OpenClaw uses strict host-key checking for SCP, so the relay host key must already exist in `~/.ssh/known_hosts`. @@ -224,6 +226,7 @@ exec ssh -T bot@mac-mini.tailnet-1234.ts.net imsg "$@" ``` Use SSH keys so both SSH and SCP are non-interactive. + Ensure the host key is trusted first (for example `ssh bot@mac-mini.tailnet-1234.ts.net`) so `known_hosts` is populated. @@ -241,6 +244,7 @@ exec ssh -T bot@mac-mini.tailnet-1234.ts.net imsg "$@" - inbound attachment ingestion is optional: `channels.imessage.includeAttachments` - remote attachment paths can be fetched via SCP when `remoteHost` is set + - SCP uses strict host-key checking (`StrictHostKeyChecking=yes`) - outbound media size uses `channels.imessage.mediaMaxMb` (default 16 MB) @@ -326,6 +330,7 @@ openclaw channels status --probe - `channels.imessage.remoteHost` - SSH/SCP key auth from the gateway host + - host key exists in `~/.ssh/known_hosts` on the gateway host - remote path readability on the Mac running Messages diff --git a/docs/gateway/configuration-reference.md b/docs/gateway/configuration-reference.md index e800690fd8..0ca00016ee 100644 --- a/docs/gateway/configuration-reference.md +++ b/docs/gateway/configuration-reference.md @@ -404,7 +404,8 @@ OpenClaw spawns `imsg rpc` (JSON-RPC over stdio). No daemon or port required. - Requires Full Disk Access to the Messages DB. - Prefer `chat_id:` targets. Use `imsg chats --limit 20` to list chats. -- `cliPath` can point to an SSH wrapper; set `remoteHost` for SCP attachment fetching. +- `cliPath` can point to an SSH wrapper; set `remoteHost` (`host` or `user@host`) for SCP attachment fetching. +- SCP uses strict host-key checking, so ensure the relay host key already exists in `~/.ssh/known_hosts`. diff --git a/docs/platforms/mac/remote.md b/docs/platforms/mac/remote.md index e24d3c5e41..71b67070c8 100644 --- a/docs/platforms/mac/remote.md +++ b/docs/platforms/mac/remote.md @@ -56,6 +56,7 @@ Remote mode supports two transports: ## Security notes - Prefer loopback binds on the remote host and connect via SSH or Tailscale. +- SSH tunneling uses strict host-key checking; trust the host key first so it exists in `~/.ssh/known_hosts`. - If you bind the Gateway to a non-loopback interface, require token/password auth. - See [Security](/gateway/security) and [Tailscale](/gateway/tailscale). diff --git a/src/auto-reply/reply/stage-sandbox-media.ts b/src/auto-reply/reply/stage-sandbox-media.ts index 43d289da5e..3147dbe8d4 100644 --- a/src/auto-reply/reply/stage-sandbox-media.ts +++ b/src/auto-reply/reply/stage-sandbox-media.ts @@ -2,13 +2,14 @@ import { spawn } from "node:child_process"; import fs from "node:fs/promises"; import path from "node:path"; import { fileURLToPath } from "node:url"; +import type { OpenClawConfig } from "../../config/config.js"; +import type { MsgContext, TemplateContext } from "../templating.js"; import { assertSandboxPath } from "../../agents/sandbox-paths.js"; import { ensureSandboxWorkspaceForSession } from "../../agents/sandbox.js"; -import type { OpenClawConfig } from "../../config/config.js"; import { logVerbose } from "../../globals.js"; +import { normalizeScpRemoteHost } from "../../infra/scp-host.js"; import { getMediaDir } from "../../media/store.js"; import { CONFIG_DIR } from "../../utils.js"; -import type { MsgContext, TemplateContext } from "../templating.js"; export async function stageSandboxMedia(params: { ctx: MsgContext; @@ -165,6 +166,10 @@ export async function stageSandboxMedia(params: { } async function scpFile(remoteHost: string, remotePath: string, localPath: string): Promise { + const safeRemoteHost = normalizeScpRemoteHost(remoteHost); + if (!safeRemoteHost) { + throw new Error("invalid remote host for SCP"); + } return new Promise((resolve, reject) => { const child = spawn( "/usr/bin/scp", @@ -172,8 +177,9 @@ async function scpFile(remoteHost: string, remotePath: string, localPath: string "-o", "BatchMode=yes", "-o", - "StrictHostKeyChecking=accept-new", - `${remoteHost}:${remotePath}`, + "StrictHostKeyChecking=yes", + "--", + `${safeRemoteHost}:${remotePath}`, localPath, ], { stdio: ["ignore", "ignore", "pipe"] }, diff --git a/src/config/config.schema-regressions.test.ts b/src/config/config.schema-regressions.test.ts index ffe204bc68..0639bc3eb0 100644 --- a/src/config/config.schema-regressions.test.ts +++ b/src/config/config.schema-regressions.test.ts @@ -36,4 +36,31 @@ describe("config schema regressions", () => { expect(res.ok).toBe(true); }); + + it("accepts safe iMessage remoteHost", () => { + const res = validateConfigObject({ + channels: { + imessage: { + remoteHost: "bot@gateway-host", + }, + }, + }); + + expect(res.ok).toBe(true); + }); + + it("rejects unsafe iMessage remoteHost", () => { + const res = validateConfigObject({ + channels: { + imessage: { + remoteHost: "bot@gateway-host -oProxyCommand=whoami", + }, + }, + }); + + expect(res.ok).toBe(false); + if (!res.ok) { + expect(res.issues[0]?.path).toBe("channels.imessage.remoteHost"); + } + }); }); diff --git a/src/config/types.imessage.ts b/src/config/types.imessage.ts index e8b4b69e2c..fc4fd4d54b 100644 --- a/src/config/types.imessage.ts +++ b/src/config/types.imessage.ts @@ -23,7 +23,7 @@ export type IMessageAccountConfig = { cliPath?: string; /** Optional Messages db path override. */ dbPath?: string; - /** Remote host for SCP when attachments live on a different machine (e.g., openclaw@192.168.64.3). */ + /** Remote SSH host token for SCP attachment fetches (`host` or `user@host`). */ remoteHost?: string; /** Optional default send service (imessage|sms|auto). */ service?: "imessage" | "sms" | "auto"; diff --git a/src/config/zod-schema.providers-core.ts b/src/config/zod-schema.providers-core.ts index 6a97c0eb92..24ac98c584 100644 --- a/src/config/zod-schema.providers-core.ts +++ b/src/config/zod-schema.providers-core.ts @@ -1,4 +1,5 @@ import { z } from "zod"; +import { isSafeScpRemoteHost } from "../infra/scp-host.js"; import { normalizeTelegramCommandDescription, normalizeTelegramCommandName, @@ -804,7 +805,10 @@ export const IMessageAccountSchemaBase = z configWrites: z.boolean().optional(), cliPath: ExecutableTokenSchema.optional(), dbPath: z.string().optional(), - remoteHost: z.string().optional(), + remoteHost: z + .string() + .refine(isSafeScpRemoteHost, "expected SSH host or user@host (no spaces/options)") + .optional(), service: z.union([z.literal("imessage"), z.literal("sms"), z.literal("auto")]).optional(), region: z.string().optional(), dmPolicy: DmPolicySchema.optional().default("pairing"), diff --git a/src/imessage/monitor/monitor-provider.ts b/src/imessage/monitor/monitor-provider.ts index 06ba9d1820..8a8abce155 100644 --- a/src/imessage/monitor/monitor-provider.ts +++ b/src/imessage/monitor/monitor-provider.ts @@ -1,4 +1,5 @@ import fs from "node:fs/promises"; +import type { IMessagePayload, MonitorIMessageOpts } from "./types.js"; import { resolveHumanDelayConfig } from "../../agents/identity.js"; import { resolveTextChunkLimit } from "../../auto-reply/chunk.js"; import { hasControlCommand } from "../../auto-reply/command-detection.js"; @@ -18,6 +19,7 @@ import { recordInboundSession } from "../../channels/session.js"; import { loadConfig } from "../../config/config.js"; import { readSessionUpdatedAt, resolveStorePath } from "../../config/sessions.js"; import { danger, logVerbose, shouldLogVerbose } from "../../globals.js"; +import { normalizeScpRemoteHost } from "../../infra/scp-host.js"; import { waitForTransportReady } from "../../infra/transport-ready.js"; import { mediaKindFromMime } from "../../media/constants.js"; import { buildPairingReply } from "../../pairing/pairing-messages.js"; @@ -39,7 +41,6 @@ import { } from "./inbound-processing.js"; import { parseIMessageNotification } from "./parse-notification.js"; import { normalizeAllowList, resolveRuntime } from "./runtime.js"; -import type { IMessagePayload, MonitorIMessageOpts } from "./types.js"; /** * Try to detect remote host from an SSH wrapper script like: @@ -146,10 +147,21 @@ export async function monitorIMessageProvider(opts: MonitorIMessageOpts = {}): P const dbPath = opts.dbPath ?? imessageCfg.dbPath; const probeTimeoutMs = imessageCfg.probeTimeoutMs ?? DEFAULT_IMESSAGE_PROBE_TIMEOUT_MS; - // Resolve remoteHost: explicit config, or auto-detect from SSH wrapper script - let remoteHost = imessageCfg.remoteHost; + // Resolve remoteHost: explicit config, or auto-detect from SSH wrapper script. + // Accept only a safe host token to avoid option/argument injection into SCP. + const configuredRemoteHost = normalizeScpRemoteHost(imessageCfg.remoteHost); + if (imessageCfg.remoteHost && !configuredRemoteHost) { + logVerbose("imessage: ignoring unsafe channels.imessage.remoteHost value"); + } + + let remoteHost = configuredRemoteHost; if (!remoteHost && cliPath && cliPath !== "imsg") { - remoteHost = await detectRemoteHostFromCliPath(cliPath); + const detected = await detectRemoteHostFromCliPath(cliPath); + const normalizedDetected = normalizeScpRemoteHost(detected); + if (detected && !normalizedDetected) { + logVerbose("imessage: ignoring unsafe auto-detected remoteHost from cliPath"); + } + remoteHost = normalizedDetected; if (remoteHost) { logVerbose(`imessage: detected remoteHost=${remoteHost} from cliPath`); } diff --git a/src/infra/scp-host.test.ts b/src/infra/scp-host.test.ts new file mode 100644 index 0000000000..178c738adf --- /dev/null +++ b/src/infra/scp-host.test.ts @@ -0,0 +1,19 @@ +import { describe, expect, it } from "vitest"; +import { isSafeScpRemoteHost, normalizeScpRemoteHost } from "./scp-host.js"; + +describe("scp remote host", () => { + it("accepts host and user@host forms", () => { + expect(normalizeScpRemoteHost("gateway-host")).toBe("gateway-host"); + expect(normalizeScpRemoteHost("bot@gateway-host")).toBe("bot@gateway-host"); + expect(normalizeScpRemoteHost("bot@192.168.64.3")).toBe("bot@192.168.64.3"); + expect(normalizeScpRemoteHost("bot@[fe80::1]")).toBe("bot@[fe80::1]"); + }); + + it("rejects unsafe host tokens", () => { + expect(isSafeScpRemoteHost("-oProxyCommand=whoami")).toBe(false); + expect(isSafeScpRemoteHost("bot@gateway-host -oStrictHostKeyChecking=no")).toBe(false); + expect(isSafeScpRemoteHost("bot@host:22")).toBe(false); + expect(isSafeScpRemoteHost("bot@/tmp/host")).toBe(false); + expect(isSafeScpRemoteHost("bot@@host")).toBe(false); + }); +}); diff --git a/src/infra/scp-host.ts b/src/infra/scp-host.ts new file mode 100644 index 0000000000..fb81171af4 --- /dev/null +++ b/src/infra/scp-host.ts @@ -0,0 +1,62 @@ +const SSH_TOKEN = /^[A-Za-z0-9._-]+$/; +const BRACKETED_IPV6 = /^\[[0-9A-Fa-f:.%]+\]$/; +const WHITESPACE = /\s/; + +function hasControlOrWhitespace(value: string): boolean { + for (const char of value) { + const code = char.charCodeAt(0); + if (code <= 0x1f || code === 0x7f || WHITESPACE.test(char)) { + return true; + } + } + return false; +} + +export function normalizeScpRemoteHost(value: string | null | undefined): string | undefined { + if (typeof value !== "string") { + return undefined; + } + const trimmed = value.trim(); + if (!trimmed) { + return undefined; + } + if (hasControlOrWhitespace(trimmed)) { + return undefined; + } + if (trimmed.startsWith("-") || trimmed.includes("/") || trimmed.includes("\\")) { + return undefined; + } + + const firstAt = trimmed.indexOf("@"); + const lastAt = trimmed.lastIndexOf("@"); + + let user: string | undefined; + let host = trimmed; + + if (firstAt !== -1) { + if (firstAt !== lastAt || firstAt === 0 || firstAt === trimmed.length - 1) { + return undefined; + } + user = trimmed.slice(0, firstAt); + host = trimmed.slice(firstAt + 1); + if (!SSH_TOKEN.test(user)) { + return undefined; + } + } + + if (!host || host.startsWith("-") || host.includes("@")) { + return undefined; + } + if (host.includes(":") && !BRACKETED_IPV6.test(host)) { + return undefined; + } + if (!SSH_TOKEN.test(host) && !BRACKETED_IPV6.test(host)) { + return undefined; + } + + return user ? `${user}@${host}` : host; +} + +export function isSafeScpRemoteHost(value: string | null | undefined): boolean { + return normalizeScpRemoteHost(value) !== undefined; +} diff --git a/src/infra/ssh-tunnel.ts b/src/infra/ssh-tunnel.ts index 391bf2bcd3..7f50f095bb 100644 --- a/src/infra/ssh-tunnel.ts +++ b/src/infra/ssh-tunnel.ts @@ -135,7 +135,7 @@ export async function startSshPortForward(opts: { "-o", "BatchMode=yes", "-o", - "StrictHostKeyChecking=accept-new", + "StrictHostKeyChecking=yes", "-o", "UpdateHostKeys=yes", "-o",