diff --git a/CHANGELOG.md b/CHANGELOG.md index 1568496abf9..b1856e72159 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ Docs: https://docs.openclaw.ai +## Unreleased + ## 2026.2.22 (Unreleased) ### Changes @@ -29,6 +31,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- Config/Memory: allow `"mistral"` in `agents.defaults.memorySearch.provider` and `agents.defaults.memorySearch.fallback` schema validation. (#14934) Thanks @ThomsenDrake. - Security/Feishu: enforce ID-only allowlist matching for DM/group sender authorization, normalize Feishu ID prefixes during checks, and ignore mutable display names so display-name collisions cannot satisfy allowlist entries. This ships in the next npm release. Thanks @jiseoung for reporting. - Feishu/Commands: in group chats, command authorization now falls back to top-level `channels.feishu.allowFrom` when per-group `allowFrom` is not set, so `/command` no longer gets blocked by an unintended empty allowlist. (#23756) - Feishu/Plugins: restore bundled Feishu SDK availability for global installs and strip `openclaw: workspace:*` from plugin `devDependencies` during plugin-version sync so npm-installed Feishu plugins do not fail dependency install. (#23611, #23645, #23603) @@ -56,6 +59,7 @@ Docs: https://docs.openclaw.ai - Telegram/Polling: clear Telegram webhooks (`deleteWebhook`) before starting long-poll `getUpdates`, including retry handling for transient cleanup failures. - Telegram/Webhook: add `channels.telegram.webhookPort` config support and pass it through plugin startup wiring to the monitor listener. - Telegram/Media: send a user-facing Telegram reply when media download fails (non-size errors) instead of silently dropping the message. +- Agents/Media: route tool-result `MEDIA:` extraction through shared parser validation so malformed prose like `MEDIA:-prefixed ...` is no longer treated as a local file path (prevents Telegram ENOENT tool-error overrides). (#18780) Thanks @HOYALIM. - Logging: cap single log-file size with `logging.maxFileBytes` (default 500 MB) and suppress additional writes after cap hit to prevent disk exhaustion from repeated error storms. - Memory/Remote HTTP: centralize remote memory HTTP calls behind a shared guarded helper (`withRemoteHttpResponse`) so embeddings and batch flows use one request/release path. - Memory/Embeddings: apply configured remote-base host pinning (`allowedHostnames`) across OpenAI/Voyage/Gemini embedding requests to keep private/self-hosted endpoints working without cross-host drift. (#18198) Thanks @ianpcook. @@ -79,8 +83,11 @@ Docs: https://docs.openclaw.ai - Channels/Delivery: remove hardcoded WhatsApp delivery fallbacks; require explicit/session channel context or auto-pick the sole configured channel when unambiguous. (#23357) Thanks @lbo728. - ACP/Gateway: wait for gateway hello before opening ACP requests, and fail fast on pre-hello connect failures to avoid startup hangs and early `gateway not connected` request races. (#23390) Thanks @janckerchen. - Gateway/Auth: preserve `OPENCLAW_GATEWAY_PASSWORD` env override precedence for remote gateway call credentials after shared resolver refactors, preventing stale configured remote passwords from overriding runtime secret rotation. +- Gateway/Auth: preserve shared-token `gateway token mismatch` auth errors when `auth.token` fallback device-token checks fail, and reserve `device token mismatch` guidance for explicit `auth.deviceToken` failures. - Gateway/Tools: when agent tools pass an allowlisted `gatewayUrl` override, resolve local override tokens from env/config fallback but keep remote overrides strict to `gateway.remote.token`, preventing local token leakage to remote targets. - Gateway/Client: keep cached device-auth tokens on `device token mismatch` closes when the client used explicit shared token/password credentials, avoiding accidental pairing-token churn during explicit-auth failures. +- Node host/Exec: keep strict Windows allowlist behavior for `cmd.exe /c` shell-wrapper runs, and return explicit approval guidance when blocked (`SYSTEM_RUN_DENIED: allowlist miss`). +- Control UI: show pairing-required guidance (commands + mobile tokenized URL reminder) when the dashboard disconnects with `1008 pairing required`. - Security/Audit: add `openclaw security audit` detection for open group policies that expose runtime/filesystem tools without sandbox/workspace guards (`security.exposure.open_groups_with_runtime_or_fs`). - Security/Audit: make `gateway.real_ip_fallback_enabled` severity conditional for loopback trusted-proxy setups (warn for loopback-only `trustedProxies`, critical when non-loopback proxies are trusted). (#23428) Thanks @bmendonca3. - Security/Exec env: block request-scoped `HOME` and `ZDOTDIR` overrides in host exec env sanitizers (Node + macOS), preventing shell startup-file execution before allowlist-evaluated command bodies. This ships in the next npm release. Thanks @tdjackey for reporting. @@ -95,6 +102,8 @@ Docs: https://docs.openclaw.ai - Telegram/Network: default Node 22+ DNS result ordering to `ipv4first` for Telegram fetch paths and add `OPENCLAW_TELEGRAM_DNS_RESULT_ORDER`/`channels.telegram.network.dnsResultOrder` overrides to reduce IPv6-path fetch failures. (#5405) Thanks @Glucksberg. - Telegram/Forward bursts: coalesce forwarded text+media updates through a dedicated forward lane debounce window that works with default inbound debounce config, while keeping forwarded control commands immediate. (#19476) thanks @napetrov. - Telegram/Streaming: preserve archived draft preview mapping after flush and clean superseded reasoning preview bubbles so multi-message preview finals no longer cross-edit or orphan stale messages under send/rotation races. (#23202) Thanks @obviyus. +- Telegram/Replies: scope messaging-tool text/media dedupe to same-target sends only, so cross-target tool sends can no longer silently suppress Telegram final replies. +- Telegram/Replies: normalize `file://` and local-path media variants during messaging dedupe so equivalent media paths do not produce duplicate Telegram replies. - Telegram/Replies: extract forwarded-origin context from unified reply targets (`reply_to_message` and `external_reply`) so forward+comment metadata is preserved across partial reply shapes. (#9720) thanks @mcaxtr. - Telegram/Polling: persist a safe update-offset watermark bounded by pending updates so crash/restart cannot skip queued lower `update_id` updates after out-of-order completion. (#23284) thanks @frankekn. - Telegram/Polling: force-restart stuck runner instances when recoverable unhandled network rejections escape the polling task path, so polling resumes instead of silently stalling. (#19721) Thanks @jg-noncelogic. @@ -116,6 +125,7 @@ Docs: https://docs.openclaw.ai - Agents/Fallbacks: treat JSON payloads with `type: "api_error"` + `"Internal server error"` as transient failover errors so Anthropic 500-style failures trigger model fallback. (#23193) Thanks @jarvis-lane. - Agents/Google: sanitize non-base64 `thought_signature`/`thoughtSignature` values from assistant replay transcripts for native Google Gemini requests while preserving valid signatures and tool-call order. (#23457) Thanks @echoVic. - Agents/Transcripts: validate assistant tool-call names (syntax/length + registered tool allowlist) before transcript persistence and during replay sanitization so malformed failover tool names no longer poison sessions with repeated provider HTTP 400 errors. (#23324) Thanks @johnsantry. +- Agents/Mistral: sanitize tool-call IDs in the embedded agent loop and generate strict provider-safe pending tool-call IDs, preventing Mistral strict9 `HTTP 400` failures on tool continuations. (#23698) Thanks @echoVic. - Agents/Compaction: strip stale assistant usage snapshots from pre-compaction turns when replaying history after a compaction summary so context-token estimation no longer reuses pre-compaction totals and immediately re-triggers destructive follow-up compactions. (#19127) Thanks @tedwatson. - Agents/Replies: emit a default completion acknowledgement (`✅ Done.`) when runs execute tools successfully but return no final assistant text, preventing silent no-reply turns after tool-only completions. (#22834) Thanks @Oldshue. - Agents/Subagents: honor `tools.subagents.tools.alsoAllow` and explicit subagent `allow` entries when resolving built-in subagent deny defaults, so explicitly granted tools (for example `sessions_send`) are no longer blocked unless re-denied in `tools.subagents.tools.deny`. (#23359) Thanks @goren-beehero. diff --git a/docs/cli/nodes.md b/docs/cli/nodes.md index 59c8a342d35..1bc8fd90c2c 100644 --- a/docs/cli/nodes.md +++ b/docs/cli/nodes.md @@ -69,5 +69,7 @@ Flags: - `--invoke-timeout `: node invoke timeout (default `30000`). - `--needs-screen-recording`: require screen recording permission. - `--raw `: run a shell string (`/bin/sh -lc` or `cmd.exe /c`). + In allowlist mode on Windows node hosts, `cmd.exe /c` shell-wrapper runs require approval + (allowlist entry alone does not auto-allow the wrapper form). - `--agent `: agent-scoped approvals/allowlists (defaults to configured agent). - `--ask `, `--security `: overrides. diff --git a/docs/nodes/index.md b/docs/nodes/index.md index 430d07299db..21dd651f554 100644 --- a/docs/nodes/index.md +++ b/docs/nodes/index.md @@ -279,6 +279,7 @@ Notes: - `system.notify` respects notification permission state on the macOS app. - `system.run` supports `--cwd`, `--env KEY=VAL`, `--command-timeout`, and `--needs-screen-recording`. - For shell wrappers (`bash|sh|zsh ... -c/-lc`), request-scoped `--env` values are reduced to an explicit allowlist (`TERM`, `LANG`, `LC_*`, `COLORTERM`, `NO_COLOR`, `FORCE_COLOR`). +- On Windows node hosts in allowlist mode, shell-wrapper runs via `cmd.exe /c` require approval (allowlist entry alone does not auto-allow the wrapper form). - `system.notify` supports `--priority ` and `--delivery `. - Node hosts ignore `PATH` overrides and strip dangerous startup/shell keys (`DYLD_*`, `LD_*`, `NODE_OPTIONS`, `PYTHON*`, `PERL*`, `RUBYOPT`, `SHELLOPTS`, `PS4`). If you need extra PATH entries, configure the node host service environment (or install tools in standard locations) instead of passing `PATH` via `--env`. - On macOS node mode, `system.run` is gated by exec approvals in the macOS app (Settings → Exec approvals). diff --git a/docs/nodes/troubleshooting.md b/docs/nodes/troubleshooting.md index ce815cdf00e..c8ba10bac49 100644 --- a/docs/nodes/troubleshooting.md +++ b/docs/nodes/troubleshooting.md @@ -86,6 +86,8 @@ If pairing is fine but `system.run` fails, fix exec approvals/allowlist. - `LOCATION_BACKGROUND_UNAVAILABLE` → app is backgrounded but only While Using permission exists. - `SYSTEM_RUN_DENIED: approval required` → exec request needs explicit approval. - `SYSTEM_RUN_DENIED: allowlist miss` → command blocked by allowlist mode. + On Windows node hosts, shell-wrapper forms like `cmd.exe /c ...` are treated as allowlist misses in + allowlist mode unless approved via ask flow. ## Fast recovery loop diff --git a/src/agents/pi-embedded-helpers/errors.ts b/src/agents/pi-embedded-helpers/errors.ts index 9e0ceb050de..68ee31f3fa5 100644 --- a/src/agents/pi-embedded-helpers/errors.ts +++ b/src/agents/pi-embedded-helpers/errors.ts @@ -614,6 +614,7 @@ const ERROR_PATTERNS = { "tool_use_id", "messages.1.content.1.tool_use.id", "invalid request format", + /tool call id was.*must be/i, ], } as const; diff --git a/src/agents/pi-embedded-runner.test.ts b/src/agents/pi-embedded-runner.test.ts index 84f48af9722..b28b43360a9 100644 --- a/src/agents/pi-embedded-runner.test.ts +++ b/src/agents/pi-embedded-runner.test.ts @@ -5,6 +5,23 @@ import "./test-helpers/fast-coding-tools.js"; import { afterAll, beforeAll, describe, expect, it, vi } from "vitest"; import type { OpenClawConfig } from "../config/config.js"; +function createMockUsage(input: number, output: number) { + return { + input, + output, + cacheRead: 0, + cacheWrite: 0, + totalTokens: input + output, + cost: { + input: 0, + output: 0, + cacheRead: 0, + cacheWrite: 0, + total: 0, + }, + }; +} + vi.mock("@mariozechner/pi-coding-agent", async () => { const actual = await vi.importActual( "@mariozechner/pi-coding-agent", @@ -40,20 +57,7 @@ vi.mock("@mariozechner/pi-ai", async () => { api: model.api, provider: model.provider, model: model.id, - usage: { - input: 1, - output: 1, - cacheRead: 0, - cacheWrite: 0, - totalTokens: 2, - cost: { - input: 0, - output: 0, - cacheRead: 0, - cacheWrite: 0, - total: 0, - }, - }, + usage: createMockUsage(1, 1), timestamp: Date.now(), }); @@ -65,20 +69,7 @@ vi.mock("@mariozechner/pi-ai", async () => { api: model.api, provider: model.provider, model: model.id, - usage: { - input: 0, - output: 0, - cacheRead: 0, - cacheWrite: 0, - totalTokens: 0, - cost: { - input: 0, - output: 0, - cacheRead: 0, - cacheWrite: 0, - total: 0, - }, - }, + usage: createMockUsage(0, 0), timestamp: Date.now(), }); @@ -314,20 +305,7 @@ describe.concurrent("runEmbeddedPiAgent", () => { api: "openai-responses", provider: "openai", model: "mock-1", - usage: { - input: 1, - output: 1, - cacheRead: 0, - cacheWrite: 0, - totalTokens: 2, - cost: { - input: 0, - output: 0, - cacheRead: 0, - cacheWrite: 0, - total: 0, - }, - }, + usage: createMockUsage(1, 1), timestamp: Date.now(), }); diff --git a/src/agents/pi-embedded-runner/extra-params.openrouter-cache-control.test.ts b/src/agents/pi-embedded-runner/extra-params.openrouter-cache-control.test.ts index 15cd10f5e79..71af916ccac 100644 --- a/src/agents/pi-embedded-runner/extra-params.openrouter-cache-control.test.ts +++ b/src/agents/pi-embedded-runner/extra-params.openrouter-cache-control.test.ts @@ -4,6 +4,32 @@ import { createAssistantMessageEventStream } from "@mariozechner/pi-ai"; import { describe, expect, it } from "vitest"; import { applyExtraParamsToAgent } from "./extra-params.js"; +type StreamPayload = { + messages: Array<{ + role: string; + content: unknown; + }>; +}; + +function runOpenRouterPayload(payload: StreamPayload, modelId: string) { + const baseStreamFn: StreamFn = (_model, _context, options) => { + options?.onPayload?.(payload); + return createAssistantMessageEventStream(); + }; + const agent = { streamFn: baseStreamFn }; + + applyExtraParamsToAgent(agent, undefined, "openrouter", modelId); + + const model = { + api: "openai-completions", + provider: "openrouter", + id: modelId, + } as Model<"openai-completions">; + const context: Context = { messages: [] }; + + void agent.streamFn?.(model, context, {}); +} + describe("extra-params: OpenRouter Anthropic cache_control", () => { it("injects cache_control into system message for OpenRouter Anthropic models", () => { const payload = { @@ -12,22 +38,8 @@ describe("extra-params: OpenRouter Anthropic cache_control", () => { { role: "user", content: "Hello" }, ], }; - const baseStreamFn: StreamFn = (_model, _context, options) => { - options?.onPayload?.(payload); - return createAssistantMessageEventStream(); - }; - const agent = { streamFn: baseStreamFn }; - applyExtraParamsToAgent(agent, undefined, "openrouter", "anthropic/claude-opus-4-6"); - - const model = { - api: "openai-completions", - provider: "openrouter", - id: "anthropic/claude-opus-4-6", - } as Model<"openai-completions">; - const context: Context = { messages: [] }; - - void agent.streamFn?.(model, context, {}); + runOpenRouterPayload(payload, "anthropic/claude-opus-4-6"); expect(payload.messages[0].content).toEqual([ { type: "text", text: "You are a helpful assistant.", cache_control: { type: "ephemeral" } }, @@ -47,22 +59,8 @@ describe("extra-params: OpenRouter Anthropic cache_control", () => { }, ], }; - const baseStreamFn: StreamFn = (_model, _context, options) => { - options?.onPayload?.(payload); - return createAssistantMessageEventStream(); - }; - const agent = { streamFn: baseStreamFn }; - applyExtraParamsToAgent(agent, undefined, "openrouter", "anthropic/claude-opus-4-6"); - - const model = { - api: "openai-completions", - provider: "openrouter", - id: "anthropic/claude-opus-4-6", - } as Model<"openai-completions">; - const context: Context = { messages: [] }; - - void agent.streamFn?.(model, context, {}); + runOpenRouterPayload(payload, "anthropic/claude-opus-4-6"); const content = payload.messages[0].content as Array>; expect(content[0]).toEqual({ type: "text", text: "Part 1" }); @@ -77,23 +75,19 @@ describe("extra-params: OpenRouter Anthropic cache_control", () => { const payload = { messages: [{ role: "system", content: "You are a helpful assistant." }], }; - const baseStreamFn: StreamFn = (_model, _context, options) => { - options?.onPayload?.(payload); - return createAssistantMessageEventStream(); - }; - const agent = { streamFn: baseStreamFn }; - applyExtraParamsToAgent(agent, undefined, "openrouter", "google/gemini-3-pro"); - - const model = { - api: "openai-completions", - provider: "openrouter", - id: "google/gemini-3-pro", - } as Model<"openai-completions">; - const context: Context = { messages: [] }; - - void agent.streamFn?.(model, context, {}); + runOpenRouterPayload(payload, "google/gemini-3-pro"); expect(payload.messages[0].content).toBe("You are a helpful assistant."); }); + + it("leaves payload unchanged when no system message exists", () => { + const payload = { + messages: [{ role: "user", content: "Hello" }], + }; + + runOpenRouterPayload(payload, "anthropic/claude-opus-4-6"); + + expect(payload.messages[0].content).toBe("Hello"); + }); }); diff --git a/src/agents/pi-embedded-runner/run.ts b/src/agents/pi-embedded-runner/run.ts index 0a810a38a6d..1347354e3c4 100644 --- a/src/agents/pi-embedded-runner/run.ts +++ b/src/agents/pi-embedded-runner/run.ts @@ -1,3 +1,4 @@ +import { randomBytes } from "node:crypto"; import fs from "node:fs/promises"; import type { ThinkLevel } from "../../auto-reply/thinking.js"; import { generateSecureToken } from "../../infra/secure-random.js"; @@ -1122,7 +1123,7 @@ export async function runEmbeddedPiAgent( pendingToolCalls: attempt.clientToolCall ? [ { - id: `call_${Date.now()}`, + id: randomBytes(5).toString("hex").slice(0, 9), name: attempt.clientToolCall.name, arguments: JSON.stringify(attempt.clientToolCall.params), }, diff --git a/src/agents/pi-embedded-runner/run/attempt.ts b/src/agents/pi-embedded-runner/run/attempt.ts index d8ba48e1564..d0892148bc2 100644 --- a/src/agents/pi-embedded-runner/run/attempt.ts +++ b/src/agents/pi-embedded-runner/run/attempt.ts @@ -77,6 +77,7 @@ import { } from "../../skills.js"; import { buildSystemPromptParams } from "../../system-prompt-params.js"; import { buildSystemPromptReport } from "../../system-prompt-report.js"; +import { sanitizeToolCallIdsForCloudCodeAssist } from "../../tool-call-id.js"; import { resolveTranscriptPolicy } from "../../transcript-policy.js"; import { DEFAULT_BOOTSTRAP_FILENAME } from "../../workspace.js"; import { isRunnerAbortError } from "../abort.js"; @@ -771,6 +772,32 @@ export async function runEmbeddedAttempt( }; } + // Mistral (and other strict providers) reject tool call IDs that don't match their + // format requirements (e.g. [a-zA-Z0-9]{9}). sanitizeSessionHistory only processes + // historical messages at attempt start, but the agent loop's internal tool call → + // tool result cycles bypass that path. Wrap streamFn so every outbound request + // sees sanitized tool call IDs. + if (transcriptPolicy.sanitizeToolCallIds && transcriptPolicy.toolCallIdMode) { + const inner = activeSession.agent.streamFn; + const mode = transcriptPolicy.toolCallIdMode; + activeSession.agent.streamFn = (model, context, options) => { + const ctx = context as unknown as { messages?: unknown }; + const messages = ctx?.messages; + if (!Array.isArray(messages)) { + return inner(model, context, options); + } + const sanitized = sanitizeToolCallIdsForCloudCodeAssist(messages as AgentMessage[], mode); + if (sanitized === messages) { + return inner(model, context, options); + } + const nextContext = { + ...(context as unknown as Record), + messages: sanitized, + } as unknown; + return inner(model, nextContext as typeof context, options); + }; + } + if (anthropicPayloadLogger) { activeSession.agent.streamFn = anthropicPayloadLogger.wrapStreamFn( activeSession.agent.streamFn, diff --git a/src/agents/pi-embedded-subscribe.tools.media.test.ts b/src/agents/pi-embedded-subscribe.tools.media.test.ts index 3452830f271..a07ed71473d 100644 --- a/src/agents/pi-embedded-subscribe.tools.media.test.ts +++ b/src/agents/pi-embedded-subscribe.tools.media.test.ts @@ -175,6 +175,18 @@ describe("extractToolResultMediaPaths", () => { expect(extractToolResultMediaPaths(result)).toEqual([]); }); + it("does not treat malformed MEDIA:-prefixed prose as a file path", () => { + const result = { + content: [ + { + type: "text", + text: "MEDIA:-prefixed paths (lenient whitespace) when loading outbound media", + }, + ], + }; + expect(extractToolResultMediaPaths(result)).toEqual([]); + }); + it("still extracts MEDIA: at line start after other text lines", () => { const result = { content: [ diff --git a/src/agents/pi-embedded-subscribe.tools.ts b/src/agents/pi-embedded-subscribe.tools.ts index 996e0c10c6c..f162d0cbd76 100644 --- a/src/agents/pi-embedded-subscribe.tools.ts +++ b/src/agents/pi-embedded-subscribe.tools.ts @@ -1,6 +1,6 @@ import { getChannelPlugin, normalizeChannelId } from "../channels/plugins/index.js"; import { normalizeTargetForProvider } from "../infra/outbound/target-normalization.js"; -import { MEDIA_TOKEN_RE } from "../media/parse.js"; +import { splitMediaFromOutput } from "../media/parse.js"; import { truncateUtf16Safe } from "../utils.js"; import { collectTextContentBlocks } from "./content-blocks.js"; import { type MessagingToolSend } from "./pi-embedded-messaging.js"; @@ -203,7 +203,8 @@ export function extractToolResultMediaPaths(result: unknown): string[] { return []; } - // Extract MEDIA: paths from text content blocks. + // Extract MEDIA: paths from text content blocks using the shared parser so + // directive matching and validation stay in sync with outbound reply parsing. const paths: string[] = []; let hasImageContent = false; for (const item of content) { @@ -216,24 +217,9 @@ export function extractToolResultMediaPaths(result: unknown): string[] { continue; } if (entry.type === "text" && typeof entry.text === "string") { - // Only parse lines that start with MEDIA: (after trimming) to avoid - // false-matching placeholders like or mid-line mentions. - // Mirrors the line-start guard in splitMediaFromOutput (media/parse.ts). - for (const line of entry.text.split("\n")) { - if (!line.trimStart().startsWith("MEDIA:")) { - continue; - } - MEDIA_TOKEN_RE.lastIndex = 0; - let match: RegExpExecArray | null; - while ((match = MEDIA_TOKEN_RE.exec(line)) !== null) { - const p = match[1] - ?.replace(/^[`"'[{(]+/, "") - .replace(/[`"'\]})\\,]+$/, "") - .trim(); - if (p && p.length <= 4096) { - paths.push(p); - } - } + const parsed = splitMediaFromOutput(entry.text); + if (parsed.mediaUrls?.length) { + paths.push(...parsed.mediaUrls); } } } diff --git a/src/auto-reply/reply/agent-runner-payloads.test.ts b/src/auto-reply/reply/agent-runner-payloads.test.ts index a8238969585..88f7d41a4c9 100644 --- a/src/auto-reply/reply/agent-runner-payloads.test.ts +++ b/src/auto-reply/reply/agent-runner-payloads.test.ts @@ -43,4 +43,32 @@ describe("buildReplyPayloads media filter integration", () => { // Text filter removes the payload entirely (text matched), so nothing remains. expect(replyPayloads).toHaveLength(0); }); + + it("does not dedupe text for cross-target messaging sends", () => { + const { replyPayloads } = buildReplyPayloads({ + ...baseParams, + payloads: [{ text: "hello world!" }], + messageProvider: "telegram", + originatingTo: "telegram:123", + messagingToolSentTexts: ["hello world!"], + messagingToolSentTargets: [{ tool: "discord", provider: "discord", to: "channel:C1" }], + }); + + expect(replyPayloads).toHaveLength(1); + expect(replyPayloads[0]?.text).toBe("hello world!"); + }); + + it("does not dedupe media for cross-target messaging sends", () => { + const { replyPayloads } = buildReplyPayloads({ + ...baseParams, + payloads: [{ text: "photo", mediaUrl: "file:///tmp/photo.jpg" }], + messageProvider: "telegram", + originatingTo: "telegram:123", + messagingToolSentMediaUrls: ["file:///tmp/photo.jpg"], + messagingToolSentTargets: [{ tool: "slack", provider: "slack", to: "channel:C1" }], + }); + + expect(replyPayloads).toHaveLength(1); + expect(replyPayloads[0]?.mediaUrl).toBe("file:///tmp/photo.jpg"); + }); }); diff --git a/src/auto-reply/reply/agent-runner-payloads.ts b/src/auto-reply/reply/agent-runner-payloads.ts index ddc3bb0b154..a1de8c1d163 100644 --- a/src/auto-reply/reply/agent-runner-payloads.ts +++ b/src/auto-reply/reply/agent-runner-payloads.ts @@ -91,14 +91,24 @@ export function buildReplyPayloads(params: { originatingTo: params.originatingTo, accountId: params.accountId, }); - const dedupedPayloads = filterMessagingToolDuplicates({ - payloads: replyTaggedPayloads, - sentTexts: messagingToolSentTexts, - }); - const mediaFilteredPayloads = filterMessagingToolMediaDuplicates({ - payloads: dedupedPayloads, - sentMediaUrls: params.messagingToolSentMediaUrls ?? [], - }); + // Only dedupe against messaging tool sends for the same origin target. + // Cross-target sends (for example posting to another channel) must not + // suppress the current conversation's final reply. + // If target metadata is unavailable, keep legacy dedupe behavior. + const dedupeMessagingToolPayloads = + suppressMessagingToolReplies || messagingToolSentTargets.length === 0; + const dedupedPayloads = dedupeMessagingToolPayloads + ? filterMessagingToolDuplicates({ + payloads: replyTaggedPayloads, + sentTexts: messagingToolSentTexts, + }) + : replyTaggedPayloads; + const mediaFilteredPayloads = dedupeMessagingToolPayloads + ? filterMessagingToolMediaDuplicates({ + payloads: dedupedPayloads, + sentMediaUrls: params.messagingToolSentMediaUrls ?? [], + }) + : dedupedPayloads; // Filter out payloads already sent via pipeline or directly during tool flush. const filteredPayloads = shouldDropFinalPayloads ? [] diff --git a/src/auto-reply/reply/agent-runner.misc.runreplyagent.test.ts b/src/auto-reply/reply/agent-runner.misc.runreplyagent.test.ts index 66dac19a2e0..5d04655525c 100644 --- a/src/auto-reply/reply/agent-runner.misc.runreplyagent.test.ts +++ b/src/auto-reply/reply/agent-runner.misc.runreplyagent.test.ts @@ -876,6 +876,19 @@ describe("runReplyAgent messaging tool suppression", () => { expect(result).toMatchObject({ text: "hello world!" }); }); + it("keeps final reply when text matches a cross-target messaging send", async () => { + runEmbeddedPiAgentMock.mockResolvedValueOnce({ + payloads: [{ text: "hello world!" }], + messagingToolSentTexts: ["hello world!"], + messagingToolSentTargets: [{ tool: "discord", provider: "discord", to: "channel:C1" }], + meta: {}, + }); + + const result = await createRun("slack"); + + expect(result).toMatchObject({ text: "hello world!" }); + }); + it("delivers replies when account ids do not match", async () => { runEmbeddedPiAgentMock.mockResolvedValueOnce({ payloads: [{ text: "hello world!" }], diff --git a/src/auto-reply/reply/reply-payloads.test.ts b/src/auto-reply/reply/reply-payloads.test.ts index 160eed93aa6..0c52903a98c 100644 --- a/src/auto-reply/reply/reply-payloads.test.ts +++ b/src/auto-reply/reply/reply-payloads.test.ts @@ -58,4 +58,20 @@ describe("filterMessagingToolMediaDuplicates", () => { }); expect(result).toBe(payloads); }); + + it("dedupes equivalent file and local path variants", () => { + const result = filterMessagingToolMediaDuplicates({ + payloads: [{ text: "hello", mediaUrl: "/tmp/photo.jpg" }], + sentMediaUrls: ["file:///tmp/photo.jpg"], + }); + expect(result).toEqual([{ text: "hello", mediaUrl: undefined, mediaUrls: undefined }]); + }); + + it("dedupes encoded file:// paths against local paths", () => { + const result = filterMessagingToolMediaDuplicates({ + payloads: [{ text: "hello", mediaUrl: "/tmp/photo one.jpg" }], + sentMediaUrls: ["file:///tmp/photo%20one.jpg"], + }); + expect(result).toEqual([{ text: "hello", mediaUrl: undefined, mediaUrls: undefined }]); + }); }); diff --git a/src/auto-reply/reply/reply-payloads.ts b/src/auto-reply/reply/reply-payloads.ts index 5c320d502f0..41906f1227f 100644 --- a/src/auto-reply/reply/reply-payloads.ts +++ b/src/auto-reply/reply/reply-payloads.ts @@ -100,16 +100,35 @@ export function filterMessagingToolMediaDuplicates(params: { payloads: ReplyPayload[]; sentMediaUrls: string[]; }): ReplyPayload[] { + const normalizeMediaForDedupe = (value: string): string => { + const trimmed = value.trim(); + if (!trimmed) { + return ""; + } + if (!trimmed.toLowerCase().startsWith("file://")) { + return trimmed; + } + try { + const parsed = new URL(trimmed); + if (parsed.protocol === "file:") { + return decodeURIComponent(parsed.pathname || ""); + } + } catch { + // Keep fallback below for non-URL-like inputs. + } + return trimmed.replace(/^file:\/\//i, ""); + }; + const { payloads, sentMediaUrls } = params; if (sentMediaUrls.length === 0) { return payloads; } - const sentSet = new Set(sentMediaUrls); + const sentSet = new Set(sentMediaUrls.map(normalizeMediaForDedupe).filter(Boolean)); return payloads.map((payload) => { const mediaUrl = payload.mediaUrl; const mediaUrls = payload.mediaUrls; - const stripSingle = mediaUrl && sentSet.has(mediaUrl); - const filteredUrls = mediaUrls?.filter((u) => !sentSet.has(u)); + const stripSingle = mediaUrl && sentSet.has(normalizeMediaForDedupe(mediaUrl)); + const filteredUrls = mediaUrls?.filter((u) => !sentSet.has(normalizeMediaForDedupe(u))); if (!stripSingle && (!mediaUrls || filteredUrls?.length === mediaUrls.length)) { return payload; // No change } diff --git a/src/browser/bridge-server.auth.test.ts b/src/browser/bridge-server.auth.test.ts index 38e1ff51f49..685f43b060a 100644 --- a/src/browser/bridge-server.auth.test.ts +++ b/src/browser/bridge-server.auth.test.ts @@ -35,6 +35,23 @@ function buildResolvedConfig(): ResolvedBrowserConfig { describe("startBrowserBridgeServer auth", () => { const servers: Array<{ stop: () => Promise }> = []; + async function expectAuthFlow( + authConfig: { authToken?: string; authPassword?: string }, + headers: Record, + ) { + const bridge = await startBrowserBridgeServer({ + resolved: buildResolvedConfig(), + ...authConfig, + }); + servers.push({ stop: () => stopBrowserBridgeServer(bridge.server) }); + + const unauth = await fetch(`${bridge.baseUrl}/`); + expect(unauth.status).toBe(401); + + const authed = await fetch(`${bridge.baseUrl}/`, { headers }); + expect(authed.status).toBe(200); + } + afterEach(async () => { while (servers.length) { const s = servers.pop(); @@ -45,35 +62,14 @@ describe("startBrowserBridgeServer auth", () => { }); it("rejects unauthenticated requests when authToken is set", async () => { - const bridge = await startBrowserBridgeServer({ - resolved: buildResolvedConfig(), - authToken: "secret-token", - }); - servers.push({ stop: () => stopBrowserBridgeServer(bridge.server) }); - - const unauth = await fetch(`${bridge.baseUrl}/`); - expect(unauth.status).toBe(401); - - const authed = await fetch(`${bridge.baseUrl}/`, { - headers: { Authorization: "Bearer secret-token" }, - }); - expect(authed.status).toBe(200); + await expectAuthFlow({ authToken: "secret-token" }, { Authorization: "Bearer secret-token" }); }); it("accepts x-openclaw-password when authPassword is set", async () => { - const bridge = await startBrowserBridgeServer({ - resolved: buildResolvedConfig(), - authPassword: "secret-password", - }); - servers.push({ stop: () => stopBrowserBridgeServer(bridge.server) }); - - const unauth = await fetch(`${bridge.baseUrl}/`); - expect(unauth.status).toBe(401); - - const authed = await fetch(`${bridge.baseUrl}/`, { - headers: { "x-openclaw-password": "secret-password" }, - }); - expect(authed.status).toBe(200); + await expectAuthFlow( + { authPassword: "secret-password" }, + { "x-openclaw-password": "secret-password" }, + ); }); it("requires auth params", async () => { diff --git a/src/browser/chrome.default-browser.test.ts b/src/browser/chrome.default-browser.test.ts index d81ad878616..ccfdb2fc19f 100644 --- a/src/browser/chrome.default-browser.test.ts +++ b/src/browser/chrome.default-browser.test.ts @@ -17,33 +17,42 @@ import { execFileSync } from "node:child_process"; import * as fs from "node:fs"; describe("browser default executable detection", () => { - beforeEach(() => { - vi.clearAllMocks(); - }); + const launchServicesPlist = "com.apple.launchservices.secure.plist"; + const chromeExecutablePath = "/Applications/Google Chrome.app/Contents/MacOS/Google Chrome"; - it("prefers default Chromium browser on macOS", () => { + function mockMacDefaultBrowser(bundleId: string, appPath = ""): void { vi.mocked(execFileSync).mockImplementation((cmd, args) => { const argsStr = Array.isArray(args) ? args.join(" ") : ""; if (cmd === "/usr/bin/plutil" && argsStr.includes("LSHandlers")) { - return JSON.stringify([ - { LSHandlerURLScheme: "http", LSHandlerRoleAll: "com.google.Chrome" }, - ]); + return JSON.stringify([{ LSHandlerURLScheme: "http", LSHandlerRoleAll: bundleId }]); } if (cmd === "/usr/bin/osascript" && argsStr.includes("path to application id")) { - return "/Applications/Google Chrome.app"; + return appPath; } if (cmd === "/usr/bin/defaults") { return "Google Chrome"; } return ""; }); + } + + function mockChromeExecutableExists(): void { vi.mocked(fs.existsSync).mockImplementation((p) => { const value = String(p); - if (value.includes("com.apple.launchservices.secure.plist")) { + if (value.includes(launchServicesPlist)) { return true; } - return value.includes("/Applications/Google Chrome.app/Contents/MacOS/Google Chrome"); + return value.includes(chromeExecutablePath); }); + } + + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("prefers default Chromium browser on macOS", () => { + mockMacDefaultBrowser("com.google.Chrome", "/Applications/Google Chrome.app"); + mockChromeExecutableExists(); const exe = resolveBrowserExecutableForPlatform( {} as Parameters[0], @@ -55,22 +64,8 @@ describe("browser default executable detection", () => { }); it("falls back when default browser is non-Chromium on macOS", () => { - vi.mocked(execFileSync).mockImplementation((cmd, args) => { - const argsStr = Array.isArray(args) ? args.join(" ") : ""; - if (cmd === "/usr/bin/plutil" && argsStr.includes("LSHandlers")) { - return JSON.stringify([ - { LSHandlerURLScheme: "http", LSHandlerRoleAll: "com.apple.Safari" }, - ]); - } - return ""; - }); - vi.mocked(fs.existsSync).mockImplementation((p) => { - const value = String(p); - if (value.includes("com.apple.launchservices.secure.plist")) { - return true; - } - return value.includes("Google Chrome.app/Contents/MacOS/Google Chrome"); - }); + mockMacDefaultBrowser("com.apple.Safari"); + mockChromeExecutableExists(); const exe = resolveBrowserExecutableForPlatform( {} as Parameters[0], diff --git a/src/browser/chrome.test.ts b/src/browser/chrome.test.ts index 1f57e72d6a9..84839e98ce0 100644 --- a/src/browser/chrome.test.ts +++ b/src/browser/chrome.test.ts @@ -22,6 +22,15 @@ async function readJson(filePath: string): Promise> { return JSON.parse(raw) as Record; } +async function readDefaultProfileFromLocalState( + userDataDir: string, +): Promise> { + const localState = await readJson(path.join(userDataDir, "Local State")); + const profile = localState.profile as Record; + const infoCache = profile.info_cache as Record; + return infoCache.Default as Record; +} + describe("browser chrome profile decoration", () => { let fixtureRoot = ""; let fixtureCount = 0; @@ -53,10 +62,7 @@ describe("browser chrome profile decoration", () => { const expectedSignedArgb = ((0xff << 24) | 0xff4500) >> 0; - const localState = await readJson(path.join(userDataDir, "Local State")); - const profile = localState.profile as Record; - const infoCache = profile.info_cache as Record; - const def = infoCache.Default as Record; + const def = await readDefaultProfileFromLocalState(userDataDir); expect(def.name).toBe(DEFAULT_OPENCLAW_BROWSER_PROFILE_NAME); expect(def.shortcut_name).toBe(DEFAULT_OPENCLAW_BROWSER_PROFILE_NAME); @@ -84,10 +90,7 @@ describe("browser chrome profile decoration", () => { it("best-effort writes name when color is invalid", async () => { const userDataDir = await createUserDataDir(); decorateOpenClawProfile(userDataDir, { color: "lobster-orange" }); - const localState = await readJson(path.join(userDataDir, "Local State")); - const profile = localState.profile as Record; - const infoCache = profile.info_cache as Record; - const def = infoCache.Default as Record; + const def = await readDefaultProfileFromLocalState(userDataDir); expect(def.name).toBe(DEFAULT_OPENCLAW_BROWSER_PROFILE_NAME); expect(def.profile_color_seed).toBeUndefined(); diff --git a/src/channels/plugins/plugins-core.test.ts b/src/channels/plugins/plugins-core.test.ts index d7108070737..37ab09f6432 100644 --- a/src/channels/plugins/plugins-core.test.ts +++ b/src/channels/plugins/plugins-core.test.ts @@ -14,6 +14,7 @@ import type { TelegramProbe } from "../../telegram/probe.js"; import type { TelegramTokenResolution } from "../../telegram/token.js"; import { createChannelTestPluginBase, + createMSTeamsTestPluginBase, createOutboundTestPlugin, createTestRegistry, } from "../../test-utils/channel-plugins.js"; @@ -131,20 +132,7 @@ const msteamsOutbound: ChannelOutboundAdapter = { }; const msteamsPlugin: ChannelPlugin = { - id: "msteams", - meta: { - id: "msteams", - label: "Microsoft Teams", - selectionLabel: "Microsoft Teams (Bot Framework)", - docsPath: "/channels/msteams", - blurb: "Bot Framework; enterprise support.", - aliases: ["teams"], - }, - capabilities: { chatTypes: ["direct"] }, - config: { - listAccountIds: () => [], - resolveAccount: () => ({}), - }, + ...createMSTeamsTestPluginBase(), outbound: msteamsOutbound, }; diff --git a/src/cli/deps.test.ts b/src/cli/deps.test.ts index 34c28cece57..3cba4d63ad8 100644 --- a/src/cli/deps.test.ts +++ b/src/cli/deps.test.ts @@ -50,6 +50,16 @@ vi.mock("../imessage/send.js", () => { }); describe("createDefaultDeps", () => { + function expectUnusedModulesNotLoaded(exclude: keyof typeof moduleLoads): void { + const keys = Object.keys(moduleLoads) as Array; + for (const key of keys) { + if (key === exclude) { + continue; + } + expect(moduleLoads[key]).not.toHaveBeenCalled(); + } + } + beforeEach(() => { vi.clearAllMocks(); }); @@ -71,11 +81,7 @@ describe("createDefaultDeps", () => { expect(moduleLoads.telegram).toHaveBeenCalledTimes(1); expect(sendFns.telegram).toHaveBeenCalledTimes(1); - expect(moduleLoads.whatsapp).not.toHaveBeenCalled(); - expect(moduleLoads.discord).not.toHaveBeenCalled(); - expect(moduleLoads.slack).not.toHaveBeenCalled(); - expect(moduleLoads.signal).not.toHaveBeenCalled(); - expect(moduleLoads.imessage).not.toHaveBeenCalled(); + expectUnusedModulesNotLoaded("telegram"); }); it("reuses module cache after first dynamic import", async () => { diff --git a/src/commands/models.list.test.ts b/src/commands/models.list.test.ts index b46d0a4a18b..5b1f6f44547 100644 --- a/src/commands/models.list.test.ts +++ b/src/commands/models.list.test.ts @@ -104,6 +104,17 @@ function makeRuntime() { }; } +function expectModelRegistryUnavailable( + runtime: ReturnType, + expectedDetail: string, +) { + expect(runtime.error).toHaveBeenCalledTimes(1); + expect(runtime.error.mock.calls[0]?.[0]).toContain("Model registry unavailable:"); + expect(runtime.error.mock.calls[0]?.[0]).toContain(expectedDetail); + expect(runtime.log).not.toHaveBeenCalled(); + expect(process.exitCode).toBe(1); +} + beforeEach(() => { previousExitCode = process.exitCode; process.exitCode = undefined; @@ -432,12 +443,8 @@ describe("models list/status", () => { const runtime = makeRuntime(); await modelsListCommand({ json: true }, runtime); - expect(runtime.error).toHaveBeenCalledTimes(1); - expect(runtime.error.mock.calls[0]?.[0]).toContain("Model registry unavailable:"); - expect(runtime.error.mock.calls[0]?.[0]).toContain("model discovery failed"); + expectModelRegistryUnavailable(runtime, "model discovery failed"); expect(runtime.error.mock.calls[0]?.[0]).not.toContain("configured models may appear missing"); - expect(runtime.log).not.toHaveBeenCalled(); - expect(process.exitCode).toBe(1); }); it("models list fails fast when registry model discovery is unavailable", async () => { @@ -452,11 +459,7 @@ describe("models list/status", () => { modelRegistryState.available = []; await modelsListCommand({ json: true }, runtime); - expect(runtime.error).toHaveBeenCalledTimes(1); - expect(runtime.error.mock.calls[0]?.[0]).toContain("Model registry unavailable:"); - expect(runtime.error.mock.calls[0]?.[0]).toContain("model discovery unavailable"); - expect(runtime.log).not.toHaveBeenCalled(); - expect(process.exitCode).toBe(1); + expectModelRegistryUnavailable(runtime, "model discovery unavailable"); }); it("loadModelRegistry throws when model discovery is unavailable", async () => { diff --git a/src/config/sessions.cache.test.ts b/src/config/sessions.cache.test.ts index cc3c6cb75a4..a77b1fdc2ea 100644 --- a/src/config/sessions.cache.test.ts +++ b/src/config/sessions.cache.test.ts @@ -9,6 +9,22 @@ import { saveSessionStore, } from "./sessions.js"; +function createSessionEntry(overrides: Partial = {}): SessionEntry { + return { + sessionId: "id-1", + updatedAt: Date.now(), + displayName: "Test Session 1", + ...overrides, + }; +} + +function createSingleSessionStore( + entry: SessionEntry = createSessionEntry(), + key = "session:1", +): Record { + return { [key]: entry }; +} + describe("Session Store Cache", () => { let fixtureRoot = ""; let caseId = 0; @@ -43,13 +59,7 @@ describe("Session Store Cache", () => { }); it("should load session store from disk on first call", async () => { - const testStore: Record = { - "session:1": { - sessionId: "id-1", - updatedAt: Date.now(), - displayName: "Test Session 1", - }, - }; + const testStore = createSingleSessionStore(); // Write test data await saveSessionStore(storePath, testStore); @@ -60,13 +70,7 @@ describe("Session Store Cache", () => { }); it("should cache session store on first load when file is unchanged", async () => { - const testStore: Record = { - "session:1": { - sessionId: "id-1", - updatedAt: Date.now(), - displayName: "Test Session 1", - }, - }; + const testStore = createSingleSessionStore(); await saveSessionStore(storePath, testStore); @@ -84,17 +88,15 @@ describe("Session Store Cache", () => { }); it("should not allow cached session mutations to leak across loads", async () => { - const testStore: Record = { - "session:1": { - sessionId: "id-1", - updatedAt: Date.now(), + const testStore = createSingleSessionStore( + createSessionEntry({ cliSessionIds: { openai: "sess-1" }, skillsSnapshot: { prompt: "skills", skills: [{ name: "alpha" }], }, - }, - }; + }), + ); await saveSessionStore(storePath, testStore); @@ -110,13 +112,7 @@ describe("Session Store Cache", () => { }); it("should refresh cache when store file changes on disk", async () => { - const testStore: Record = { - "session:1": { - sessionId: "id-1", - updatedAt: Date.now(), - displayName: "Test Session 1", - }, - }; + const testStore = createSingleSessionStore(); await saveSessionStore(storePath, testStore); @@ -138,13 +134,7 @@ describe("Session Store Cache", () => { }); it("should invalidate cache on write", async () => { - const testStore: Record = { - "session:1": { - sessionId: "id-1", - updatedAt: Date.now(), - displayName: "Test Session 1", - }, - }; + const testStore = createSingleSessionStore(); await saveSessionStore(storePath, testStore); @@ -172,13 +162,7 @@ describe("Session Store Cache", () => { process.env.OPENCLAW_SESSION_CACHE_TTL_MS = "0"; clearSessionStoreCacheForTest(); - const testStore: Record = { - "session:1": { - sessionId: "id-1", - updatedAt: Date.now(), - displayName: "Test Session 1", - }, - }; + const testStore = createSingleSessionStore(); await saveSessionStore(storePath, testStore); @@ -187,13 +171,10 @@ describe("Session Store Cache", () => { expect(loaded1).toEqual(testStore); // Modify file on disk - const modifiedStore: Record = { - "session:2": { - sessionId: "id-2", - updatedAt: Date.now(), - displayName: "Test Session 2", - }, - }; + const modifiedStore = createSingleSessionStore( + createSessionEntry({ sessionId: "id-2", displayName: "Test Session 2" }), + "session:2", + ); fs.writeFileSync(storePath, JSON.stringify(modifiedStore, null, 2)); // Second load - should read from disk (cache disabled) diff --git a/src/config/types.agent-defaults.ts b/src/config/types.agent-defaults.ts index aa3fbe41958..60b623db563 100644 --- a/src/config/types.agent-defaults.ts +++ b/src/config/types.agent-defaults.ts @@ -1,15 +1,11 @@ import type { ChannelId } from "../channels/plugins/types.js"; +import type { AgentModelConfig, AgentSandboxConfig } from "./types.agents-shared.js"; import type { BlockStreamingChunkConfig, BlockStreamingCoalesceConfig, HumanDelayConfig, TypingMode, } from "./types.base.js"; -import type { - SandboxBrowserSettings, - SandboxDockerSettings, - SandboxPruneSettings, -} from "./types.sandbox.js"; import type { MemorySearchConfig } from "./types.tools.js"; export type AgentModelEntryConfig = { @@ -248,40 +244,12 @@ export type AgentDefaultsConfig = { /** Auto-archive sub-agent sessions after N minutes (default: 60). */ archiveAfterMinutes?: number; /** Default model selection for spawned sub-agents (string or {primary,fallbacks}). */ - model?: string | { primary?: string; fallbacks?: string[] }; + model?: AgentModelConfig; /** Default thinking level for spawned sub-agents (e.g. "off", "low", "medium", "high"). */ thinking?: string; }; /** Optional sandbox settings for non-main sessions. */ - sandbox?: { - /** Enable sandboxing for sessions. */ - mode?: "off" | "non-main" | "all"; - /** - * Agent workspace access inside the sandbox. - * - "none": do not mount the agent workspace into the container; use a sandbox workspace under workspaceRoot - * - "ro": mount the agent workspace read-only; disables write/edit tools - * - "rw": mount the agent workspace read/write; enables write/edit tools - */ - workspaceAccess?: "none" | "ro" | "rw"; - /** - * Session tools visibility for sandboxed sessions. - * - "spawned": only allow session tools to target the current session and sessions spawned from it (default) - * - "all": allow session tools to target any session - */ - sessionToolsVisibility?: "spawned" | "all"; - /** Container/workspace scope for sandbox isolation. */ - scope?: "session" | "agent" | "shared"; - /** Legacy alias for scope ("session" when true, "shared" when false). */ - perSession?: boolean; - /** Root directory for sandbox workspaces. */ - workspaceRoot?: string; - /** Docker-specific sandbox settings. */ - docker?: SandboxDockerSettings; - /** Optional sandboxed browser settings. */ - browser?: SandboxBrowserSettings; - /** Auto-prune sandbox containers. */ - prune?: SandboxPruneSettings; - }; + sandbox?: AgentSandboxConfig; }; export type AgentCompactionMode = "default" | "safeguard"; diff --git a/src/config/types.agents-shared.ts b/src/config/types.agents-shared.ts new file mode 100644 index 00000000000..152c8973c11 --- /dev/null +++ b/src/config/types.agents-shared.ts @@ -0,0 +1,37 @@ +import type { + SandboxBrowserSettings, + SandboxDockerSettings, + SandboxPruneSettings, +} from "./types.sandbox.js"; + +export type AgentModelConfig = + | string + | { + /** Primary model (provider/model). */ + primary?: string; + /** Per-agent model fallbacks (provider/model). */ + fallbacks?: string[]; + }; + +export type AgentSandboxConfig = { + mode?: "off" | "non-main" | "all"; + /** Agent workspace access inside the sandbox. */ + workspaceAccess?: "none" | "ro" | "rw"; + /** + * Session tools visibility for sandboxed sessions. + * - "spawned": only allow session tools to target sessions spawned from this session (default) + * - "all": allow session tools to target any session + */ + sessionToolsVisibility?: "spawned" | "all"; + /** Container/workspace scope for sandbox isolation. */ + scope?: "session" | "agent" | "shared"; + /** Legacy alias for scope ("session" when true, "shared" when false). */ + perSession?: boolean; + workspaceRoot?: string; + /** Docker-specific sandbox settings. */ + docker?: SandboxDockerSettings; + /** Optional sandboxed browser settings. */ + browser?: SandboxBrowserSettings; + /** Auto-prune sandbox settings. */ + prune?: SandboxPruneSettings; +}; diff --git a/src/config/types.agents.ts b/src/config/types.agents.ts index 478e14e526b..11dd9bf4a2b 100644 --- a/src/config/types.agents.ts +++ b/src/config/types.agents.ts @@ -1,23 +1,10 @@ import type { ChatType } from "../channels/chat-type.js"; import type { AgentDefaultsConfig } from "./types.agent-defaults.js"; +import type { AgentModelConfig, AgentSandboxConfig } from "./types.agents-shared.js"; import type { HumanDelayConfig, IdentityConfig } from "./types.base.js"; import type { GroupChatConfig } from "./types.messages.js"; -import type { - SandboxBrowserSettings, - SandboxDockerSettings, - SandboxPruneSettings, -} from "./types.sandbox.js"; import type { AgentToolsConfig, MemorySearchConfig } from "./types.tools.js"; -export type AgentModelConfig = - | string - | { - /** Primary model (provider/model). */ - primary?: string; - /** Per-agent model fallbacks (provider/model). */ - fallbacks?: string[]; - }; - export type AgentConfig = { id: string; default?: boolean; @@ -38,30 +25,10 @@ export type AgentConfig = { /** Allow spawning sub-agents under other agent ids. Use "*" to allow any. */ allowAgents?: string[]; /** Per-agent default model for spawned sub-agents (string or {primary,fallbacks}). */ - model?: string | { primary?: string; fallbacks?: string[] }; - }; - sandbox?: { - mode?: "off" | "non-main" | "all"; - /** Agent workspace access inside the sandbox. */ - workspaceAccess?: "none" | "ro" | "rw"; - /** - * Session tools visibility for sandboxed sessions. - * - "spawned": only allow session tools to target sessions spawned from this session (default) - * - "all": allow session tools to target any session - */ - sessionToolsVisibility?: "spawned" | "all"; - /** Container/workspace scope for sandbox isolation. */ - scope?: "session" | "agent" | "shared"; - /** Legacy alias for scope ("session" when true, "shared" when false). */ - perSession?: boolean; - workspaceRoot?: string; - /** Docker-specific sandbox overrides for this agent. */ - docker?: SandboxDockerSettings; - /** Optional sandboxed browser overrides for this agent. */ - browser?: SandboxBrowserSettings; - /** Auto-prune overrides for this agent. */ - prune?: SandboxPruneSettings; + model?: AgentModelConfig; }; + /** Optional per-agent sandbox overrides. */ + sandbox?: AgentSandboxConfig; tools?: AgentToolsConfig; }; diff --git a/src/config/types.channel-messaging-common.ts b/src/config/types.channel-messaging-common.ts new file mode 100644 index 00000000000..5d927884bd6 --- /dev/null +++ b/src/config/types.channel-messaging-common.ts @@ -0,0 +1,50 @@ +import type { + BlockStreamingCoalesceConfig, + DmPolicy, + GroupPolicy, + MarkdownConfig, +} from "./types.base.js"; +import type { ChannelHeartbeatVisibilityConfig } from "./types.channels.js"; +import type { DmConfig } from "./types.messages.js"; + +export type CommonChannelMessagingConfig = { + /** Optional display name for this account (used in CLI/UI lists). */ + name?: string; + /** Optional provider capability tags used for agent/runtime guidance. */ + capabilities?: string[]; + /** Markdown formatting overrides (tables). */ + markdown?: MarkdownConfig; + /** Allow channel-initiated config writes (default: true). */ + configWrites?: boolean; + /** If false, do not start this account. Default: true. */ + enabled?: boolean; + /** Direct message access policy (default: pairing). */ + dmPolicy?: DmPolicy; + /** Optional allowlist for inbound DM senders. */ + allowFrom?: Array; + /** Default delivery target for CLI --deliver when no explicit --reply-to is provided. */ + defaultTo?: string; + /** Optional allowlist for group/channel senders. */ + groupAllowFrom?: Array; + /** Group/channel message handling policy. */ + groupPolicy?: GroupPolicy; + /** Max group/channel messages to keep as history context (0 disables). */ + historyLimit?: number; + /** Max DM turns to keep as history context. */ + dmHistoryLimit?: number; + /** Per-DM config overrides keyed by sender ID. */ + dms?: Record; + /** Outbound text chunk size (chars). */ + textChunkLimit?: number; + /** Chunking mode: "length" (default) splits by size; "newline" splits on every newline. */ + chunkMode?: "length" | "newline"; + blockStreaming?: boolean; + /** Merge streamed block replies before sending. */ + blockStreamingCoalesce?: BlockStreamingCoalesceConfig; + /** Heartbeat visibility settings for this channel. */ + heartbeat?: ChannelHeartbeatVisibilityConfig; + /** Outbound response prefix override for this channel/account. */ + responsePrefix?: string; + /** Max outbound media size in MB. */ + mediaMaxMb?: number; +}; diff --git a/src/config/types.irc.ts b/src/config/types.irc.ts index eff575d1918..61794523195 100644 --- a/src/config/types.irc.ts +++ b/src/config/types.irc.ts @@ -1,24 +1,7 @@ -import type { - BlockStreamingCoalesceConfig, - DmPolicy, - GroupPolicy, - MarkdownConfig, -} from "./types.base.js"; -import type { ChannelHeartbeatVisibilityConfig } from "./types.channels.js"; -import type { DmConfig } from "./types.messages.js"; +import type { CommonChannelMessagingConfig } from "./types.channel-messaging-common.js"; import type { GroupToolPolicyBySenderConfig, GroupToolPolicyConfig } from "./types.tools.js"; -export type IrcAccountConfig = { - /** Optional display name for this account (used in CLI/UI lists). */ - name?: string; - /** Optional provider capability tags used for agent/runtime guidance. */ - capabilities?: string[]; - /** Markdown formatting overrides (tables). */ - markdown?: MarkdownConfig; - /** Allow channel-initiated config writes (default: true). */ - configWrites?: boolean; - /** If false, do not start this IRC account. Default: true. */ - enabled?: boolean; +export type IrcAccountConfig = CommonChannelMessagingConfig & { /** IRC server hostname (example: irc.libera.chat). */ host?: string; /** IRC server port (default: 6697 with TLS, otherwise 6667). */ @@ -52,34 +35,8 @@ export type IrcAccountConfig = { }; /** Auto-join channel list at connect (example: ["#openclaw"]). */ channels?: string[]; - /** Direct message access policy (default: pairing). */ - dmPolicy?: DmPolicy; - /** Optional allowlist for inbound DM senders. */ - allowFrom?: Array; - /** Default delivery target for CLI --deliver when no explicit --reply-to is provided. */ - defaultTo?: string; - /** Optional allowlist for IRC channel senders. */ - groupAllowFrom?: Array; - /** - * Controls how channel messages are handled: - * - "open": channels bypass allowFrom; mention-gating applies - * - "disabled": block all channel messages entirely - * - "allowlist": only allow channel messages from senders in groupAllowFrom/allowFrom - */ - groupPolicy?: GroupPolicy; - /** Max channel messages to keep as history context (0 disables). */ - historyLimit?: number; - /** Max DM turns to keep as history context. */ - dmHistoryLimit?: number; - /** Per-DM config overrides keyed by sender ID. */ - dms?: Record; /** Outbound text chunk size (chars). Default: 350. */ textChunkLimit?: number; - /** Chunking mode: "length" (default) splits by size; "newline" splits on every newline. */ - chunkMode?: "length" | "newline"; - blockStreaming?: boolean; - /** Merge streamed block replies before sending. */ - blockStreamingCoalesce?: BlockStreamingCoalesceConfig; groups?: Record< string, { @@ -94,12 +51,6 @@ export type IrcAccountConfig = { >; /** Optional mention patterns specific to IRC channel messages. */ mentionPatterns?: string[]; - /** Heartbeat visibility settings for this channel. */ - heartbeat?: ChannelHeartbeatVisibilityConfig; - /** Outbound response prefix override for this channel/account. */ - responsePrefix?: string; - /** Max outbound media size in MB. */ - mediaMaxMb?: number; }; export type IrcConfig = { diff --git a/src/config/types.signal.ts b/src/config/types.signal.ts index 8103b409906..cf45fa34025 100644 --- a/src/config/types.signal.ts +++ b/src/config/types.signal.ts @@ -1,26 +1,9 @@ -import type { - BlockStreamingCoalesceConfig, - DmPolicy, - GroupPolicy, - MarkdownConfig, -} from "./types.base.js"; -import type { ChannelHeartbeatVisibilityConfig } from "./types.channels.js"; -import type { DmConfig } from "./types.messages.js"; +import type { CommonChannelMessagingConfig } from "./types.channel-messaging-common.js"; export type SignalReactionNotificationMode = "off" | "own" | "all" | "allowlist"; export type SignalReactionLevel = "off" | "ack" | "minimal" | "extensive"; -export type SignalAccountConfig = { - /** Optional display name for this account (used in CLI/UI lists). */ - name?: string; - /** Optional provider capability tags used for agent/runtime guidance. */ - capabilities?: string[]; - /** Markdown formatting overrides (tables). */ - markdown?: MarkdownConfig; - /** Allow channel-initiated config writes (default: true). */ - configWrites?: boolean; - /** If false, do not start this Signal account. Default: true. */ - enabled?: boolean; +export type SignalAccountConfig = CommonChannelMessagingConfig & { /** Optional explicit E.164 account for signal-cli. */ account?: string; /** Optional full base URL for signal-cli HTTP daemon. */ @@ -39,34 +22,8 @@ export type SignalAccountConfig = { ignoreAttachments?: boolean; ignoreStories?: boolean; sendReadReceipts?: boolean; - /** Direct message access policy (default: pairing). */ - dmPolicy?: DmPolicy; - allowFrom?: Array; - /** Default delivery target for CLI --deliver when no explicit --reply-to is provided. */ - defaultTo?: string; - /** Optional allowlist for Signal group senders (E.164). */ - groupAllowFrom?: Array; - /** - * Controls how group messages are handled: - * - "open": groups bypass allowFrom, no extra gating - * - "disabled": block all group messages - * - "allowlist": only allow group messages from senders in groupAllowFrom/allowFrom - */ - groupPolicy?: GroupPolicy; - /** Max group messages to keep as history context (0 disables). */ - historyLimit?: number; - /** Max DM turns to keep as history context. */ - dmHistoryLimit?: number; - /** Per-DM config overrides keyed by user ID. */ - dms?: Record; /** Outbound text chunk size (chars). Default: 4000. */ textChunkLimit?: number; - /** Chunking mode: "length" (default) splits by size; "newline" splits on every newline. */ - chunkMode?: "length" | "newline"; - blockStreaming?: boolean; - /** Merge streamed block replies before sending. */ - blockStreamingCoalesce?: BlockStreamingCoalesceConfig; - mediaMaxMb?: number; /** Reaction notification mode (off|own|all|allowlist). Default: own. */ reactionNotifications?: SignalReactionNotificationMode; /** Allowlist for reaction notifications when mode is allowlist. */ @@ -84,10 +41,6 @@ export type SignalAccountConfig = { * - "extensive": Agent can react liberally */ reactionLevel?: SignalReactionLevel; - /** Heartbeat visibility settings for this channel. */ - heartbeat?: ChannelHeartbeatVisibilityConfig; - /** Outbound response prefix override for this channel/account. */ - responsePrefix?: string; }; export type SignalConfig = { diff --git a/src/config/zod-schema.agent-runtime.ts b/src/config/zod-schema.agent-runtime.ts index 507ec4c0fc1..4cd90203766 100644 --- a/src/config/zod-schema.agent-runtime.ts +++ b/src/config/zod-schema.agent-runtime.ts @@ -490,7 +490,13 @@ export const MemorySearchSchema = z .strict() .optional(), provider: z - .union([z.literal("openai"), z.literal("local"), z.literal("gemini"), z.literal("voyage")]) + .union([ + z.literal("openai"), + z.literal("local"), + z.literal("gemini"), + z.literal("voyage"), + z.literal("mistral"), + ]) .optional(), remote: z .object({ @@ -516,6 +522,7 @@ export const MemorySearchSchema = z z.literal("gemini"), z.literal("local"), z.literal("voyage"), + z.literal("mistral"), z.literal("none"), ]) .optional(), diff --git a/src/gateway/server.auth.test.ts b/src/gateway/server.auth.test.ts index 22c0e472508..a9b1b97c14a 100644 --- a/src/gateway/server.auth.test.ts +++ b/src/gateway/server.auth.test.ts @@ -993,6 +993,42 @@ describe("gateway server auth/connect", () => { restoreGatewayToken(prevToken); }); + test("keeps shared token mismatch reason when token fallback device-token check fails", async () => { + const { server, ws, port, prevToken } = await startServerWithClient("secret"); + await ensurePairedDeviceTokenForCurrentIdentity(ws); + + ws.close(); + + const ws2 = await openWs(port); + const res2 = await connectReq(ws2, { token: "wrong" }); + expect(res2.ok).toBe(false); + expect(res2.error?.message ?? "").toContain("gateway token mismatch"); + expect(res2.error?.message ?? "").not.toContain("device token mismatch"); + + ws2.close(); + await server.close(); + restoreGatewayToken(prevToken); + }); + + test("reports device token mismatch when explicit auth.deviceToken is wrong", async () => { + const { server, ws, port, prevToken } = await startServerWithClient("secret"); + await ensurePairedDeviceTokenForCurrentIdentity(ws); + + ws.close(); + + const ws2 = await openWs(port); + const res2 = await connectReq(ws2, { + skipDefaultAuth: true, + deviceToken: "not-a-valid-device-token", + }); + expect(res2.ok).toBe(false); + expect(res2.error?.message ?? "").toContain("device token mismatch"); + + ws2.close(); + await server.close(); + restoreGatewayToken(prevToken); + }); + test("keeps shared-secret lockout separate from device-token auth", async () => { const { server, port, prevToken, deviceToken } = await startRateLimitedTokenServerWithPairedDeviceToken(); diff --git a/src/gateway/server.models-voicewake-misc.test.ts b/src/gateway/server.models-voicewake-misc.test.ts index 5554fd10f4c..99e3b945e8e 100644 --- a/src/gateway/server.models-voicewake-misc.test.ts +++ b/src/gateway/server.models-voicewake-misc.test.ts @@ -442,12 +442,11 @@ describe("gateway server misc", () => { await autoServer.close(); const updated = JSON.parse(await fs.readFile(configPath, "utf-8")) as Record; - const plugins = updated.plugins as Record | undefined; - const entries = plugins?.entries as Record | undefined; - const discord = entries?.discord as Record | undefined; - expect(discord?.enabled).toBe(true); - expect((updated.channels as Record | undefined)?.discord).toMatchObject({ + const channels = updated.channels as Record | undefined; + const discord = channels?.discord as Record | undefined; + expect(discord).toMatchObject({ token: "token-123", + enabled: true, }); }); diff --git a/src/gateway/server/ws-connection/auth-context.ts b/src/gateway/server/ws-connection/auth-context.ts index 4354f05000c..70cac275a86 100644 --- a/src/gateway/server/ws-connection/auth-context.ts +++ b/src/gateway/server/ws-connection/auth-context.ts @@ -17,6 +17,8 @@ type HandshakeConnectAuth = { password?: string; }; +export type DeviceTokenCandidateSource = "explicit-device-token" | "shared-token-fallback"; + export type ConnectAuthState = { authResult: GatewayAuthResult; authOk: boolean; @@ -24,6 +26,7 @@ export type ConnectAuthState = { sharedAuthOk: boolean; sharedAuthProvided: boolean; deviceTokenCandidate?: string; + deviceTokenCandidateSource?: DeviceTokenCandidateSource; }; function trimToUndefined(value: string | undefined): string | undefined { @@ -45,14 +48,19 @@ function resolveSharedConnectAuth( return { token, password }; } -function resolveDeviceTokenCandidate( - connectAuth: HandshakeConnectAuth | null | undefined, -): string | undefined { +function resolveDeviceTokenCandidate(connectAuth: HandshakeConnectAuth | null | undefined): { + token?: string; + source?: DeviceTokenCandidateSource; +} { const explicitDeviceToken = trimToUndefined(connectAuth?.deviceToken); if (explicitDeviceToken) { - return explicitDeviceToken; + return { token: explicitDeviceToken, source: "explicit-device-token" }; } - return trimToUndefined(connectAuth?.token); + const fallbackToken = trimToUndefined(connectAuth?.token); + if (!fallbackToken) { + return {}; + } + return { token: fallbackToken, source: "shared-token-fallback" }; } export async function resolveConnectAuthState(params: { @@ -67,9 +75,8 @@ export async function resolveConnectAuthState(params: { }): Promise { const sharedConnectAuth = resolveSharedConnectAuth(params.connectAuth); const sharedAuthProvided = Boolean(sharedConnectAuth); - const deviceTokenCandidate = params.hasDeviceIdentity - ? resolveDeviceTokenCandidate(params.connectAuth) - : undefined; + const { token: deviceTokenCandidate, source: deviceTokenCandidateSource } = + params.hasDeviceIdentity ? resolveDeviceTokenCandidate(params.connectAuth) : {}; const hasDeviceTokenCandidate = Boolean(deviceTokenCandidate); let authResult: GatewayAuthResult = await authorizeWsControlUiGatewayConnect({ @@ -129,5 +136,6 @@ export async function resolveConnectAuthState(params: { sharedAuthOk, sharedAuthProvided, deviceTokenCandidate, + deviceTokenCandidateSource, }; } diff --git a/src/gateway/server/ws-connection/message-handler.ts b/src/gateway/server/ws-connection/message-handler.ts index e59d852fc7d..6df1bedefb2 100644 --- a/src/gateway/server/ws-connection/message-handler.ts +++ b/src/gateway/server/ws-connection/message-handler.ts @@ -355,17 +355,23 @@ export function attachGatewayWsMessageHandler(params: { }); const device = controlUiAuthPolicy.device; - let { authResult, authOk, authMethod, sharedAuthOk, deviceTokenCandidate } = - await resolveConnectAuthState({ - resolvedAuth, - connectAuth: connectParams.auth, - hasDeviceIdentity: Boolean(device), - req: upgradeReq, - trustedProxies, - allowRealIpFallback, - rateLimiter, - clientIp, - }); + let { + authResult, + authOk, + authMethod, + sharedAuthOk, + deviceTokenCandidate, + deviceTokenCandidateSource, + } = await resolveConnectAuthState({ + resolvedAuth, + connectAuth: connectParams.auth, + hasDeviceIdentity: Boolean(device), + req: upgradeReq, + trustedProxies, + allowRealIpFallback, + rateLimiter, + clientIp, + }); const rejectUnauthorized = (failedAuth: GatewayAuthResult) => { markHandshakeFailure("unauthorized", { authMode: resolvedAuth.mode, @@ -532,7 +538,11 @@ export function attachGatewayWsMessageHandler(params: { authMethod = "device-token"; rateLimiter?.reset(clientIp, AUTH_RATE_LIMIT_SCOPE_DEVICE_TOKEN); } else { - authResult = { ok: false, reason: "device_token_mismatch" }; + const mismatchReason = + deviceTokenCandidateSource === "explicit-device-token" + ? "device_token_mismatch" + : (authResult.reason ?? "device_token_mismatch"); + authResult = { ok: false, reason: mismatchReason }; rateLimiter?.recordFailure(clientIp, AUTH_RATE_LIMIT_SCOPE_DEVICE_TOKEN); } } diff --git a/src/hooks/install.test.ts b/src/hooks/install.test.ts index e5eeb16c01e..5c0cabc141b 100644 --- a/src/hooks/install.test.ts +++ b/src/hooks/install.test.ts @@ -3,10 +3,13 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; import { afterAll, beforeEach, describe, expect, it, vi } from "vitest"; +import { expectSingleNpmPackIgnoreScriptsCall } from "../test-utils/exec-assertions.js"; import { - expectSingleNpmInstallIgnoreScriptsCall, - expectSingleNpmPackIgnoreScriptsCall, -} from "../test-utils/exec-assertions.js"; + expectInstallUsesIgnoreScripts, + expectIntegrityDriftRejected, + expectUnsupportedNpmSpec, + mockNpmPackMetadataResult, +} from "../test-utils/npm-spec-install-test-helpers.js"; import { isAddressInUseError } from "./gmail-watcher.js"; const fixtureRoot = path.join(os.tmpdir(), `openclaw-hook-install-${randomUUID()}`); @@ -60,17 +63,6 @@ function writeArchiveFixture(params: { fileName: string; contents: Buffer }) { }; } -async function expectUnsupportedNpmSpec( - install: (spec: string) => Promise<{ ok: boolean; error?: string }>, -) { - const result = await install("github:evil/evil"); - expect(result.ok).toBe(false); - if (result.ok) { - return; - } - expect(result.error).toContain("unsupported npm spec"); -} - function expectInstallFailureContains( result: Awaited>, snippets: string[], @@ -196,26 +188,13 @@ describe("installHooksFromPath", () => { ); const run = vi.mocked(runCommandWithTimeout); - run.mockResolvedValue({ - code: 0, - stdout: "", - stderr: "", - signal: null, - killed: false, - termination: "exit", - }); - - const res = await installHooksFromPath({ - path: pkgDir, - hooksDir: path.join(stateDir, "hooks"), - }); - expect(res.ok).toBe(true); - if (!res.ok) { - return; - } - expectSingleNpmInstallIgnoreScriptsCall({ - calls: run.mock.calls as Array<[unknown, { cwd?: string } | undefined]>, - expectedCwd: res.targetDir, + await expectInstallUsesIgnoreScripts({ + run, + install: async () => + await installHooksFromPath({ + path: pkgDir, + hooksDir: path.join(stateDir, "hooks"), + }), }); }); @@ -383,22 +362,13 @@ describe("installHooksFromNpmSpec", () => { it("aborts when integrity drift callback rejects the fetched artifact", async () => { const run = vi.mocked(runCommandWithTimeout); - run.mockResolvedValue({ - code: 0, - stdout: JSON.stringify([ - { - id: "@openclaw/test-hooks@0.0.1", - name: "@openclaw/test-hooks", - version: "0.0.1", - filename: "test-hooks-0.0.1.tgz", - integrity: "sha512-new", - shasum: "newshasum", - }, - ]), - stderr: "", - signal: null, - killed: false, - termination: "exit", + mockNpmPackMetadataResult(run, { + id: "@openclaw/test-hooks@0.0.1", + name: "@openclaw/test-hooks", + version: "0.0.1", + filename: "test-hooks-0.0.1.tgz", + integrity: "sha512-new", + shasum: "newshasum", }); const onIntegrityDrift = vi.fn(async () => false); @@ -407,18 +377,12 @@ describe("installHooksFromNpmSpec", () => { expectedIntegrity: "sha512-old", onIntegrityDrift, }); - - expect(onIntegrityDrift).toHaveBeenCalledWith( - expect.objectContaining({ - expectedIntegrity: "sha512-old", - actualIntegrity: "sha512-new", - }), - ); - expect(result.ok).toBe(false); - if (result.ok) { - return; - } - expect(result.error).toContain("integrity drift"); + expectIntegrityDriftRejected({ + onIntegrityDrift, + result, + expectedIntegrity: "sha512-old", + actualIntegrity: "sha512-new", + }); }); }); diff --git a/src/hooks/install.ts b/src/hooks/install.ts index a394d92d570..c6032b8247e 100644 --- a/src/hooks/install.ts +++ b/src/hooks/install.ts @@ -1,22 +1,23 @@ import fs from "node:fs/promises"; import path from "node:path"; import { MANIFEST_KEY } from "../compat/legacy-names.js"; +import { fileExists, readJsonFile, resolveArchiveKind } from "../infra/archive.js"; +import { resolveExistingInstallPath, withExtractedArchiveRoot } from "../infra/install-flow.js"; import { - extractArchive, - fileExists, - readJsonFile, - resolveArchiveKind, - resolvePackedRootDir, -} from "../infra/archive.js"; + resolveInstallModeOptions, + resolveTimedInstallModeOptions, +} from "../infra/install-mode-options.js"; import { installPackageDir } from "../infra/install-package-dir.js"; import { resolveSafeInstallDir, unscopedPackageName } from "../infra/install-safe-path.js"; import { type NpmIntegrityDrift, type NpmSpecResolution, resolveArchiveSourcePath, - withTempDir, } from "../infra/install-source-utils.js"; -import { installFromNpmSpecArchive } from "../infra/npm-pack-install.js"; +import { + finalizeNpmSpecArchiveInstall, + installFromNpmSpecArchiveWithInstaller, +} from "../infra/npm-pack-install.js"; import { validateRegistryNpmSpec } from "../infra/npm-registry-spec.js"; import { isPathInside, isPathInsideWithRealpath } from "../security/scan-paths.js"; import { CONFIG_DIR, resolveUserPath } from "../utils.js"; @@ -96,30 +97,6 @@ async function ensureOpenClawHooks(manifest: HookPackageManifest) { return list; } -function resolveHookInstallModeOptions(params: { - logger?: HookInstallLogger; - mode?: "install" | "update"; - dryRun?: boolean; -}): { logger: HookInstallLogger; mode: "install" | "update"; dryRun: boolean } { - return { - logger: params.logger ?? defaultLogger, - mode: params.mode ?? "install", - dryRun: params.dryRun ?? false, - }; -} - -function resolveTimedHookInstallModeOptions(params: { - logger?: HookInstallLogger; - timeoutMs?: number; - mode?: "install" | "update"; - dryRun?: boolean; -}): { logger: HookInstallLogger; timeoutMs: number; mode: "install" | "update"; dryRun: boolean } { - return { - ...resolveHookInstallModeOptions(params), - timeoutMs: params.timeoutMs ?? 120_000, - }; -} - async function resolveInstallTargetDir( id: string, hooksDir?: string, @@ -173,7 +150,7 @@ async function installHookPackageFromDir(params: { dryRun?: boolean; expectedHookPackId?: string; }): Promise { - const { logger, timeoutMs, mode, dryRun } = resolveTimedHookInstallModeOptions(params); + const { logger, timeoutMs, mode, dryRun } = resolveTimedInstallModeOptions(params, defaultLogger); const manifestPath = path.join(params.packageDir, "package.json"); if (!(await fileExists(manifestPath))) { @@ -283,7 +260,7 @@ async function installHookFromDir(params: { dryRun?: boolean; expectedHookPackId?: string; }): Promise { - const { logger, mode, dryRun } = resolveHookInstallModeOptions(params); + const { logger, mode, dryRun } = resolveInstallModeOptions(params, defaultLogger); await validateHookDir(params.hookDir); const hookName = await resolveHookNameFromDir(params.hookDir); @@ -353,45 +330,34 @@ export async function installHooksFromArchive(params: { } const archivePath = archivePathResult.path; - return await withTempDir("openclaw-hook-", async (tmpDir) => { - const extractDir = path.join(tmpDir, "extract"); - await fs.mkdir(extractDir, { recursive: true }); + return await withExtractedArchiveRoot({ + archivePath, + tempDirPrefix: "openclaw-hook-", + timeoutMs, + logger, + onExtracted: async (rootDir) => { + const manifestPath = path.join(rootDir, "package.json"); + if (await fileExists(manifestPath)) { + return await installHookPackageFromDir({ + packageDir: rootDir, + hooksDir: params.hooksDir, + timeoutMs, + logger, + mode: params.mode, + dryRun: params.dryRun, + expectedHookPackId: params.expectedHookPackId, + }); + } - logger.info?.(`Extracting ${archivePath}…`); - try { - await extractArchive({ archivePath, destDir: extractDir, timeoutMs, logger }); - } catch (err) { - return { ok: false, error: `failed to extract archive: ${String(err)}` }; - } - - let rootDir = ""; - try { - rootDir = await resolvePackedRootDir(extractDir); - } catch (err) { - return { ok: false, error: String(err) }; - } - - const manifestPath = path.join(rootDir, "package.json"); - if (await fileExists(manifestPath)) { - return await installHookPackageFromDir({ - packageDir: rootDir, + return await installHookFromDir({ + hookDir: rootDir, hooksDir: params.hooksDir, - timeoutMs, logger, mode: params.mode, dryRun: params.dryRun, expectedHookPackId: params.expectedHookPackId, }); - } - - return await installHookFromDir({ - hookDir: rootDir, - hooksDir: params.hooksDir, - logger, - mode: params.mode, - dryRun: params.dryRun, - expectedHookPackId: params.expectedHookPackId, - }); + }, }); } @@ -406,7 +372,7 @@ export async function installHooksFromNpmSpec(params: { expectedIntegrity?: string; onIntegrityDrift?: (params: HookNpmIntegrityDriftParams) => boolean | Promise; }): Promise { - const { logger, timeoutMs, mode, dryRun } = resolveTimedHookInstallModeOptions(params); + const { logger, timeoutMs, mode, dryRun } = resolveTimedInstallModeOptions(params, defaultLogger); const expectedHookPackId = params.expectedHookPackId; const spec = params.spec.trim(); const specError = validateRegistryNpmSpec(spec); @@ -415,7 +381,7 @@ export async function installHooksFromNpmSpec(params: { } logger.info?.(`Downloading ${spec}…`); - const flowResult = await installFromNpmSpecArchive({ + const flowResult = await installFromNpmSpecArchiveWithInstaller({ tempDirPrefix: "openclaw-hook-pack-", spec, timeoutMs, @@ -424,28 +390,17 @@ export async function installHooksFromNpmSpec(params: { warn: (message) => { logger.warn?.(message); }, - installFromArchive: async ({ archivePath }) => - await installHooksFromArchive({ - archivePath, - hooksDir: params.hooksDir, - timeoutMs, - logger, - mode, - dryRun, - expectedHookPackId, - }), + installFromArchive: installHooksFromArchive, + archiveInstallParams: { + hooksDir: params.hooksDir, + timeoutMs, + logger, + mode, + dryRun, + expectedHookPackId, + }, }); - if (!flowResult.ok) { - return flowResult; - } - if (!flowResult.installResult.ok) { - return flowResult.installResult; - } - return { - ...flowResult.installResult, - npmResolution: flowResult.npmResolution, - integrityDrift: flowResult.integrityDrift, - }; + return finalizeNpmSpecArchiveInstall(flowResult); } export async function installHooksFromPath(params: { @@ -457,12 +412,12 @@ export async function installHooksFromPath(params: { dryRun?: boolean; expectedHookPackId?: string; }): Promise { - const resolved = resolveUserPath(params.path); - if (!(await fileExists(resolved))) { - return { ok: false, error: `path not found: ${resolved}` }; + const pathResult = await resolveExistingInstallPath(params.path); + if (!pathResult.ok) { + return pathResult; } + const { resolvedPath: resolved, stat } = pathResult; - const stat = await fs.stat(resolved); if (stat.isDirectory()) { const manifestPath = path.join(resolved, "package.json"); if (await fileExists(manifestPath)) { diff --git a/src/infra/http-body.test.ts b/src/infra/http-body.test.ts index cfe6934863e..bfb14b92dca 100644 --- a/src/infra/http-body.test.ts +++ b/src/infra/http-body.test.ts @@ -113,15 +113,29 @@ describe("http body limits", () => { expect(req.__unhandledDestroyError).toBeUndefined(); }); - it("timeout surfaces typed error", async () => { + it("timeout surfaces typed error when timeoutMs is clamped", async () => { const req = createMockRequest({ emitEnd: false }); - const promise = readRequestBodyWithLimit(req, { maxBytes: 128, timeoutMs: 10 }); + const promise = readRequestBodyWithLimit(req, { maxBytes: 128, timeoutMs: 0 }); await expect(promise).rejects.toSatisfy((error: unknown) => isRequestBodyLimitError(error, "REQUEST_BODY_TIMEOUT"), ); expect(req.__unhandledDestroyError).toBeUndefined(); }); + it("guard clamps invalid maxBytes to one byte", async () => { + const req = createMockRequest({ chunks: ["ab"], emitEnd: false }); + const res = createMockServerResponse(); + const guard = installRequestBodyLimitGuard(req, res, { + maxBytes: Number.NaN, + responseFormat: "text", + }); + await waitForMicrotaskTurn(); + expect(guard.isTripped()).toBe(true); + expect(guard.code()).toBe("PAYLOAD_TOO_LARGE"); + expect(res.statusCode).toBe(413); + expect(req.__unhandledDestroyError).toBeUndefined(); + }); + it("declared oversized content-length does not emit unhandled error", async () => { const req = createMockRequest({ headers: { "content-length": "9999" }, diff --git a/src/infra/http-body.ts b/src/infra/http-body.ts index 3f7fc9c3dc1..cfa65560028 100644 --- a/src/infra/http-body.ts +++ b/src/infra/http-body.ts @@ -79,10 +79,15 @@ export type ReadRequestBodyOptions = { encoding?: BufferEncoding; }; -export async function readRequestBodyWithLimit( - req: IncomingMessage, - options: ReadRequestBodyOptions, -): Promise { +type RequestBodyLimitValues = { + maxBytes: number; + timeoutMs: number; +}; + +function resolveRequestBodyLimitValues(options: { + maxBytes: number; + timeoutMs?: number; +}): RequestBodyLimitValues { const maxBytes = Number.isFinite(options.maxBytes) ? Math.max(1, Math.floor(options.maxBytes)) : 1; @@ -90,6 +95,14 @@ export async function readRequestBodyWithLimit( typeof options.timeoutMs === "number" && Number.isFinite(options.timeoutMs) ? Math.max(1, Math.floor(options.timeoutMs)) : DEFAULT_WEBHOOK_BODY_TIMEOUT_MS; + return { maxBytes, timeoutMs }; +} + +export async function readRequestBodyWithLimit( + req: IncomingMessage, + options: ReadRequestBodyOptions, +): Promise { + const { maxBytes, timeoutMs } = resolveRequestBodyLimitValues(options); const encoding = options.encoding ?? "utf-8"; const declaredLength = parseContentLengthHeader(req); @@ -241,13 +254,7 @@ export function installRequestBodyLimitGuard( res: ServerResponse, options: RequestBodyLimitGuardOptions, ): RequestBodyLimitGuard { - const maxBytes = Number.isFinite(options.maxBytes) - ? Math.max(1, Math.floor(options.maxBytes)) - : 1; - const timeoutMs = - typeof options.timeoutMs === "number" && Number.isFinite(options.timeoutMs) - ? Math.max(1, Math.floor(options.timeoutMs)) - : DEFAULT_WEBHOOK_BODY_TIMEOUT_MS; + const { maxBytes, timeoutMs } = resolveRequestBodyLimitValues(options); const responseFormat = options.responseFormat ?? "json"; const customText = options.responseText ?? {}; diff --git a/src/infra/install-flow.test.ts b/src/infra/install-flow.test.ts new file mode 100644 index 00000000000..62148f8e075 --- /dev/null +++ b/src/infra/install-flow.test.ts @@ -0,0 +1,122 @@ +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import * as archive from "./archive.js"; +import { resolveExistingInstallPath, withExtractedArchiveRoot } from "./install-flow.js"; +import * as installSource from "./install-source-utils.js"; + +describe("resolveExistingInstallPath", () => { + let fixtureRoot = ""; + + beforeEach(async () => { + fixtureRoot = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-install-flow-")); + }); + + afterEach(async () => { + if (fixtureRoot) { + await fs.rm(fixtureRoot, { recursive: true, force: true }); + } + }); + + it("returns resolved path and stat for existing files", async () => { + const filePath = path.join(fixtureRoot, "plugin.tgz"); + await fs.writeFile(filePath, "archive"); + + const result = await resolveExistingInstallPath(filePath); + + expect(result.ok).toBe(true); + if (!result.ok) { + return; + } + expect(result.resolvedPath).toBe(filePath); + expect(result.stat.isFile()).toBe(true); + }); + + it("returns a path-not-found error for missing paths", async () => { + const missing = path.join(fixtureRoot, "missing.tgz"); + + const result = await resolveExistingInstallPath(missing); + + expect(result).toEqual({ + ok: false, + error: `path not found: ${missing}`, + }); + }); +}); + +describe("withExtractedArchiveRoot", () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it("extracts archive and passes root directory to callback", async () => { + const withTempDirSpy = vi + .spyOn(installSource, "withTempDir") + .mockImplementation(async (_prefix, fn) => await fn("/tmp/openclaw-install-flow")); + const extractSpy = vi.spyOn(archive, "extractArchive").mockResolvedValue(undefined); + const resolveRootSpy = vi + .spyOn(archive, "resolvePackedRootDir") + .mockResolvedValue("/tmp/openclaw-install-flow/extract/package"); + + const onExtracted = vi.fn(async (rootDir: string) => ({ ok: true as const, rootDir })); + const result = await withExtractedArchiveRoot({ + archivePath: "/tmp/plugin.tgz", + tempDirPrefix: "openclaw-plugin-", + timeoutMs: 1000, + onExtracted, + }); + + expect(withTempDirSpy).toHaveBeenCalledWith("openclaw-plugin-", expect.any(Function)); + expect(extractSpy).toHaveBeenCalledWith( + expect.objectContaining({ + archivePath: "/tmp/plugin.tgz", + }), + ); + expect(resolveRootSpy).toHaveBeenCalledWith("/tmp/openclaw-install-flow/extract"); + expect(onExtracted).toHaveBeenCalledWith("/tmp/openclaw-install-flow/extract/package"); + expect(result).toEqual({ + ok: true, + rootDir: "/tmp/openclaw-install-flow/extract/package", + }); + }); + + it("returns extract failure when extraction throws", async () => { + vi.spyOn(installSource, "withTempDir").mockImplementation( + async (_prefix, fn) => await fn("/tmp/openclaw-install-flow"), + ); + vi.spyOn(archive, "extractArchive").mockRejectedValue(new Error("boom")); + + const result = await withExtractedArchiveRoot({ + archivePath: "/tmp/plugin.tgz", + tempDirPrefix: "openclaw-plugin-", + timeoutMs: 1000, + onExtracted: async () => ({ ok: true as const }), + }); + + expect(result).toEqual({ + ok: false, + error: "failed to extract archive: Error: boom", + }); + }); + + it("returns root-resolution failure when archive layout is invalid", async () => { + vi.spyOn(installSource, "withTempDir").mockImplementation( + async (_prefix, fn) => await fn("/tmp/openclaw-install-flow"), + ); + vi.spyOn(archive, "extractArchive").mockResolvedValue(undefined); + vi.spyOn(archive, "resolvePackedRootDir").mockRejectedValue(new Error("invalid layout")); + + const result = await withExtractedArchiveRoot({ + archivePath: "/tmp/plugin.tgz", + tempDirPrefix: "openclaw-plugin-", + timeoutMs: 1000, + onExtracted: async () => ({ ok: true as const }), + }); + + expect(result).toEqual({ + ok: false, + error: "Error: invalid layout", + }); + }); +}); diff --git a/src/infra/install-flow.ts b/src/infra/install-flow.ts new file mode 100644 index 00000000000..17d4cad2c7f --- /dev/null +++ b/src/infra/install-flow.ts @@ -0,0 +1,61 @@ +import type { Stats } from "node:fs"; +import fs from "node:fs/promises"; +import path from "node:path"; +import { resolveUserPath } from "../utils.js"; +import { type ArchiveLogger, extractArchive, fileExists, resolvePackedRootDir } from "./archive.js"; +import { withTempDir } from "./install-source-utils.js"; + +export type ExistingInstallPathResult = + | { + ok: true; + resolvedPath: string; + stat: Stats; + } + | { + ok: false; + error: string; + }; + +export async function resolveExistingInstallPath( + inputPath: string, +): Promise { + const resolvedPath = resolveUserPath(inputPath); + if (!(await fileExists(resolvedPath))) { + return { ok: false, error: `path not found: ${resolvedPath}` }; + } + const stat = await fs.stat(resolvedPath); + return { ok: true, resolvedPath, stat }; +} + +export async function withExtractedArchiveRoot(params: { + archivePath: string; + tempDirPrefix: string; + timeoutMs: number; + logger?: ArchiveLogger; + onExtracted: (rootDir: string) => Promise; +}): Promise { + return await withTempDir(params.tempDirPrefix, async (tmpDir) => { + const extractDir = path.join(tmpDir, "extract"); + await fs.mkdir(extractDir, { recursive: true }); + + params.logger?.info?.(`Extracting ${params.archivePath}…`); + try { + await extractArchive({ + archivePath: params.archivePath, + destDir: extractDir, + timeoutMs: params.timeoutMs, + logger: params.logger, + }); + } catch (err) { + return { ok: false, error: `failed to extract archive: ${String(err)}` }; + } + + let rootDir = ""; + try { + rootDir = await resolvePackedRootDir(extractDir); + } catch (err) { + return { ok: false, error: String(err) }; + } + return await params.onExtracted(rootDir); + }); +} diff --git a/src/infra/install-mode-options.test.ts b/src/infra/install-mode-options.test.ts new file mode 100644 index 00000000000..fe9cfa1a64c --- /dev/null +++ b/src/infra/install-mode-options.test.ts @@ -0,0 +1,51 @@ +import { describe, expect, it } from "vitest"; +import { + resolveInstallModeOptions, + resolveTimedInstallModeOptions, +} from "./install-mode-options.js"; + +describe("install mode option helpers", () => { + it("applies logger, mode, and dryRun defaults", () => { + const logger = { warn: (_message: string) => {} }; + const result = resolveInstallModeOptions({}, logger); + + expect(result).toEqual({ + logger, + mode: "install", + dryRun: false, + }); + }); + + it("preserves explicit mode and dryRun values", () => { + const logger = { warn: (_message: string) => {} }; + const result = resolveInstallModeOptions( + { + logger, + mode: "update", + dryRun: true, + }, + { warn: () => {} }, + ); + + expect(result).toEqual({ + logger, + mode: "update", + dryRun: true, + }); + }); + + it("uses default timeout when not provided", () => { + const logger = { warn: (_message: string) => {} }; + const result = resolveTimedInstallModeOptions({}, logger); + + expect(result.timeoutMs).toBe(120_000); + expect(result.mode).toBe("install"); + expect(result.dryRun).toBe(false); + }); + + it("honors custom timeout default override", () => { + const result = resolveTimedInstallModeOptions({}, { warn: () => {} }, 5000); + + expect(result.timeoutMs).toBe(5000); + }); +}); diff --git a/src/infra/install-mode-options.ts b/src/infra/install-mode-options.ts new file mode 100644 index 00000000000..dabb8f5380e --- /dev/null +++ b/src/infra/install-mode-options.ts @@ -0,0 +1,42 @@ +export type InstallMode = "install" | "update"; + +export type InstallModeOptions = { + logger?: TLogger; + mode?: InstallMode; + dryRun?: boolean; +}; + +export type TimedInstallModeOptions = InstallModeOptions & { + timeoutMs?: number; +}; + +export function resolveInstallModeOptions( + params: InstallModeOptions, + defaultLogger: TLogger, +): { + logger: TLogger; + mode: InstallMode; + dryRun: boolean; +} { + return { + logger: params.logger ?? defaultLogger, + mode: params.mode ?? "install", + dryRun: params.dryRun ?? false, + }; +} + +export function resolveTimedInstallModeOptions( + params: TimedInstallModeOptions, + defaultLogger: TLogger, + defaultTimeoutMs = 120_000, +): { + logger: TLogger; + timeoutMs: number; + mode: InstallMode; + dryRun: boolean; +} { + return { + ...resolveInstallModeOptions(params, defaultLogger), + timeoutMs: params.timeoutMs ?? defaultTimeoutMs, + }; +} diff --git a/src/infra/npm-pack-install.test.ts b/src/infra/npm-pack-install.test.ts index 503b27a1b3c..c0428ec03c5 100644 --- a/src/infra/npm-pack-install.test.ts +++ b/src/infra/npm-pack-install.test.ts @@ -1,7 +1,11 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { packNpmSpecToArchive, withTempDir } from "./install-source-utils.js"; import type { NpmIntegrityDriftPayload } from "./npm-integrity.js"; -import { installFromNpmSpecArchive } from "./npm-pack-install.js"; +import { + finalizeNpmSpecArchiveInstall, + installFromNpmSpecArchive, + installFromNpmSpecArchiveWithInstaller, +} from "./npm-pack-install.js"; vi.mock("./install-source-utils.js", async (importOriginal) => { const actual = await importOriginal(); @@ -173,3 +177,99 @@ describe("installFromNpmSpecArchive", () => { expect(okResult.integrityDrift).toBeUndefined(); }); }); + +describe("installFromNpmSpecArchiveWithInstaller", () => { + beforeEach(() => { + vi.mocked(packNpmSpecToArchive).mockClear(); + }); + + it("passes archive path and installer params to installFromArchive", async () => { + vi.mocked(packNpmSpecToArchive).mockResolvedValue({ + ok: true, + archivePath: "/tmp/openclaw-plugin.tgz", + metadata: { + resolvedSpec: "@openclaw/voice-call@1.0.0", + integrity: "sha512-same", + }, + }); + const installFromArchive = vi.fn( + async (_params: { archivePath: string; pluginId: string }) => + ({ ok: true as const, pluginId: "voice-call" }) as const, + ); + + const result = await installFromNpmSpecArchiveWithInstaller({ + tempDirPrefix: "openclaw-test-", + spec: "@openclaw/voice-call@1.0.0", + timeoutMs: 1000, + installFromArchive, + archiveInstallParams: { pluginId: "voice-call" }, + }); + + expect(result.ok).toBe(true); + if (!result.ok) { + return; + } + expect(installFromArchive).toHaveBeenCalledWith({ + archivePath: "/tmp/openclaw-plugin.tgz", + pluginId: "voice-call", + }); + expect(result.installResult).toEqual({ ok: true, pluginId: "voice-call" }); + }); +}); + +describe("finalizeNpmSpecArchiveInstall", () => { + it("returns top-level flow errors unchanged", () => { + const result = finalizeNpmSpecArchiveInstall<{ ok: true } | { ok: false; error: string }>({ + ok: false, + error: "pack failed", + }); + + expect(result).toEqual({ ok: false, error: "pack failed" }); + }); + + it("returns install errors unchanged", () => { + const result = finalizeNpmSpecArchiveInstall<{ ok: true } | { ok: false; error: string }>({ + ok: true, + installResult: { ok: false, error: "install failed" }, + npmResolution: { + resolvedSpec: "@openclaw/test@1.0.0", + integrity: "sha512-same", + resolvedAt: "2026-01-01T00:00:00.000Z", + }, + }); + + expect(result).toEqual({ ok: false, error: "install failed" }); + }); + + it("attaches npm metadata to successful install results", () => { + const result = finalizeNpmSpecArchiveInstall< + { ok: true; pluginId: string } | { ok: false; error: string } + >({ + ok: true, + installResult: { ok: true, pluginId: "voice-call" }, + npmResolution: { + resolvedSpec: "@openclaw/voice-call@1.0.0", + integrity: "sha512-same", + resolvedAt: "2026-01-01T00:00:00.000Z", + }, + integrityDrift: { + expectedIntegrity: "sha512-old", + actualIntegrity: "sha512-same", + }, + }); + + expect(result).toEqual({ + ok: true, + pluginId: "voice-call", + npmResolution: { + resolvedSpec: "@openclaw/voice-call@1.0.0", + integrity: "sha512-same", + resolvedAt: "2026-01-01T00:00:00.000Z", + }, + integrityDrift: { + expectedIntegrity: "sha512-old", + actualIntegrity: "sha512-same", + }, + }); + }); +}); diff --git a/src/infra/npm-pack-install.ts b/src/infra/npm-pack-install.ts index 6663c416993..447563c3015 100644 --- a/src/infra/npm-pack-install.ts +++ b/src/infra/npm-pack-install.ts @@ -21,6 +21,58 @@ export type NpmSpecArchiveInstallFlowResult = integrityDrift?: NpmIntegrityDrift; }; +export async function installFromNpmSpecArchiveWithInstaller< + TResult extends { ok: boolean }, + TArchiveInstallParams extends { archivePath: string }, +>(params: { + tempDirPrefix: string; + spec: string; + timeoutMs: number; + expectedIntegrity?: string; + onIntegrityDrift?: (payload: NpmIntegrityDriftPayload) => boolean | Promise; + warn?: (message: string) => void; + installFromArchive: (params: TArchiveInstallParams) => Promise; + archiveInstallParams: Omit; +}): Promise> { + return await installFromNpmSpecArchive({ + tempDirPrefix: params.tempDirPrefix, + spec: params.spec, + timeoutMs: params.timeoutMs, + expectedIntegrity: params.expectedIntegrity, + onIntegrityDrift: params.onIntegrityDrift, + warn: params.warn, + installFromArchive: async ({ archivePath }) => + await params.installFromArchive({ + archivePath, + ...params.archiveInstallParams, + } as TArchiveInstallParams), + }); +} + +export type NpmSpecArchiveFinalInstallResult = + | { ok: false; error: string } + | Exclude + | (Extract & { + npmResolution: NpmSpecResolution; + integrityDrift?: NpmIntegrityDrift; + }); + +export function finalizeNpmSpecArchiveInstall( + flowResult: NpmSpecArchiveInstallFlowResult, +): NpmSpecArchiveFinalInstallResult { + if (!flowResult.ok) { + return flowResult; + } + if (!flowResult.installResult.ok) { + return flowResult.installResult; + } + return { + ...flowResult.installResult, + npmResolution: flowResult.npmResolution, + integrityDrift: flowResult.integrityDrift, + }; +} + export async function installFromNpmSpecArchive(params: { tempDirPrefix: string; spec: string; diff --git a/src/node-host/invoke-system-run.test.ts b/src/node-host/invoke-system-run.test.ts new file mode 100644 index 00000000000..424bad83ed6 --- /dev/null +++ b/src/node-host/invoke-system-run.test.ts @@ -0,0 +1,16 @@ +import { describe, expect, it } from "vitest"; +import { formatSystemRunAllowlistMissMessage } from "./invoke-system-run.js"; + +describe("formatSystemRunAllowlistMissMessage", () => { + it("returns legacy allowlist miss message by default", () => { + expect(formatSystemRunAllowlistMissMessage()).toBe("SYSTEM_RUN_DENIED: allowlist miss"); + }); + + it("adds Windows shell-wrapper guidance when blocked by cmd.exe policy", () => { + expect( + formatSystemRunAllowlistMissMessage({ + windowsShellWrapperBlocked: true, + }), + ).toContain("Windows shell wrappers like cmd.exe /c require approval"); + }); +}); diff --git a/src/node-host/invoke-system-run.ts b/src/node-host/invoke-system-run.ts index 92f3b632b62..87df62926ae 100644 --- a/src/node-host/invoke-system-run.ts +++ b/src/node-host/invoke-system-run.ts @@ -33,6 +33,19 @@ type SystemRunInvokeResult = { error?: { code?: string; message?: string } | null; }; +export function formatSystemRunAllowlistMissMessage(params?: { + windowsShellWrapperBlocked?: boolean; +}): string { + if (params?.windowsShellWrapperBlocked) { + return ( + "SYSTEM_RUN_DENIED: allowlist miss " + + "(Windows shell wrappers like cmd.exe /c require approval; " + + "approve once/always or run with --ask on-miss|always)" + ); + } + return "SYSTEM_RUN_DENIED: allowlist miss"; +} + export async function handleSystemRunInvoke(opts: { client: GatewayClient; params: SystemRunParams; @@ -163,7 +176,8 @@ export async function handleSystemRunInvoke(opts: { const cmdInvocation = shellCommand ? opts.isCmdExeInvocation(segments[0]?.argv ?? []) : opts.isCmdExeInvocation(argv); - if (security === "allowlist" && isWindows && cmdInvocation) { + const windowsShellWrapperBlocked = security === "allowlist" && isWindows && cmdInvocation; + if (windowsShellWrapperBlocked) { analysisOk = false; allowlistSatisfied = false; } @@ -317,7 +331,10 @@ export async function handleSystemRunInvoke(opts: { ); await opts.sendInvokeResult({ ok: false, - error: { code: "UNAVAILABLE", message: "SYSTEM_RUN_DENIED: allowlist miss" }, + error: { + code: "UNAVAILABLE", + message: formatSystemRunAllowlistMissMessage({ windowsShellWrapperBlocked }), + }, }); return; } diff --git a/src/plugins/install.test.ts b/src/plugins/install.test.ts index a2642acfba4..02360ebccc6 100644 --- a/src/plugins/install.test.ts +++ b/src/plugins/install.test.ts @@ -6,10 +6,13 @@ import JSZip from "jszip"; import * as tar from "tar"; import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; import * as skillScanner from "../security/skill-scanner.js"; +import { expectSingleNpmPackIgnoreScriptsCall } from "../test-utils/exec-assertions.js"; import { - expectSingleNpmInstallIgnoreScriptsCall, - expectSingleNpmPackIgnoreScriptsCall, -} from "../test-utils/exec-assertions.js"; + expectInstallUsesIgnoreScripts, + expectIntegrityDriftRejected, + expectUnsupportedNpmSpec, + mockNpmPackMetadataResult, +} from "../test-utils/npm-spec-install-test-helpers.js"; vi.mock("../process/exec.js", () => ({ runCommandWithTimeout: vi.fn(), @@ -181,20 +184,37 @@ async function expectArchiveInstallReservedSegmentRejection(params: { packageName: string; outName: string; }) { - const stateDir = makeTempDir(); - const workDir = makeTempDir(); - const pkgDir = path.join(workDir, "package"); - fs.mkdirSync(path.join(pkgDir, "dist"), { recursive: true }); - fs.writeFileSync( - path.join(pkgDir, "package.json"), - JSON.stringify({ + const result = await installArchivePackageAndReturnResult({ + packageJson: { name: params.packageName, version: "0.0.1", openclaw: { extensions: ["./dist/index.js"] }, - }), - "utf-8", - ); - fs.writeFileSync(path.join(pkgDir, "dist", "index.js"), "export {};", "utf-8"); + }, + outName: params.outName, + withDistIndex: true, + }); + + expect(result.ok).toBe(false); + if (result.ok) { + return; + } + expect(result.error).toContain("reserved path segment"); +} + +async function installArchivePackageAndReturnResult(params: { + packageJson: Record; + outName: string; + withDistIndex?: boolean; +}) { + const stateDir = makeTempDir(); + const workDir = makeTempDir(); + const pkgDir = path.join(workDir, "package"); + fs.mkdirSync(pkgDir, { recursive: true }); + if (params.withDistIndex) { + fs.mkdirSync(path.join(pkgDir, "dist"), { recursive: true }); + fs.writeFileSync(path.join(pkgDir, "dist", "index.js"), "export {};", "utf-8"); + } + fs.writeFileSync(path.join(pkgDir, "package.json"), JSON.stringify(params.packageJson), "utf-8"); const archivePath = await packToArchive({ pkgDir, @@ -207,12 +227,7 @@ async function expectArchiveInstallReservedSegmentRejection(params: { archivePath, extensionsDir, }); - - expect(result.ok).toBe(false); - if (result.ok) { - return; - } - expect(result.error).toContain("reserved path segment"); + return result; } afterAll(() => { @@ -346,27 +361,10 @@ describe("installPluginFromArchive", () => { }); it("rejects packages without openclaw.extensions", async () => { - const stateDir = makeTempDir(); - const workDir = makeTempDir(); - const pkgDir = path.join(workDir, "package"); - fs.mkdirSync(pkgDir, { recursive: true }); - fs.writeFileSync( - path.join(pkgDir, "package.json"), - JSON.stringify({ name: "@openclaw/nope", version: "0.0.1" }), - "utf-8", - ); - - const archivePath = await packToArchive({ - pkgDir, - outDir: workDir, + const result = await installArchivePackageAndReturnResult({ + packageJson: { name: "@openclaw/nope", version: "0.0.1" }, outName: "bad.tgz", }); - - const extensionsDir = path.join(stateDir, "extensions"); - const result = await installPluginFromArchive({ - archivePath, - extensionsDir, - }); expect(result.ok).toBe(false); if (result.ok) { return; @@ -464,26 +462,13 @@ describe("installPluginFromDir", () => { fs.writeFileSync(path.join(pluginDir, "dist", "index.js"), "export {};", "utf-8"); const run = vi.mocked(runCommandWithTimeout); - run.mockResolvedValue({ - code: 0, - stdout: "", - stderr: "", - signal: null, - killed: false, - termination: "exit", - }); - - const res = await installPluginFromDir({ - dirPath: pluginDir, - extensionsDir: path.join(stateDir, "extensions"), - }); - expect(res.ok).toBe(true); - if (!res.ok) { - return; - } - expectSingleNpmInstallIgnoreScriptsCall({ - calls: run.mock.calls as Array<[unknown, { cwd?: string } | undefined]>, - expectedCwd: res.targetDir, + await expectInstallUsesIgnoreScripts({ + run, + install: async () => + await installPluginFromDir({ + dirPath: pluginDir, + extensionsDir: path.join(stateDir, "extensions"), + }), }); }); @@ -596,32 +581,18 @@ describe("installPluginFromNpmSpec", () => { }); it("rejects non-registry npm specs", async () => { - const result = await installPluginFromNpmSpec({ spec: "github:evil/evil" }); - expect(result.ok).toBe(false); - if (result.ok) { - return; - } - expect(result.error).toContain("unsupported npm spec"); + await expectUnsupportedNpmSpec((spec) => installPluginFromNpmSpec({ spec })); }); it("aborts when integrity drift callback rejects the fetched artifact", async () => { const run = vi.mocked(runCommandWithTimeout); - run.mockResolvedValue({ - code: 0, - stdout: JSON.stringify([ - { - id: "@openclaw/voice-call@0.0.1", - name: "@openclaw/voice-call", - version: "0.0.1", - filename: "voice-call-0.0.1.tgz", - integrity: "sha512-new", - shasum: "newshasum", - }, - ]), - stderr: "", - signal: null, - killed: false, - termination: "exit", + mockNpmPackMetadataResult(run, { + id: "@openclaw/voice-call@0.0.1", + name: "@openclaw/voice-call", + version: "0.0.1", + filename: "voice-call-0.0.1.tgz", + integrity: "sha512-new", + shasum: "newshasum", }); const onIntegrityDrift = vi.fn(async () => false); @@ -630,17 +601,11 @@ describe("installPluginFromNpmSpec", () => { expectedIntegrity: "sha512-old", onIntegrityDrift, }); - - expect(onIntegrityDrift).toHaveBeenCalledWith( - expect.objectContaining({ - expectedIntegrity: "sha512-old", - actualIntegrity: "sha512-new", - }), - ); - expect(result.ok).toBe(false); - if (result.ok) { - return; - } - expect(result.error).toContain("integrity drift"); + expectIntegrityDriftRejected({ + onIntegrityDrift, + result, + expectedIntegrity: "sha512-old", + actualIntegrity: "sha512-new", + }); }); }); diff --git a/src/plugins/install.ts b/src/plugins/install.ts index 500162af703..40aeb3c5a63 100644 --- a/src/plugins/install.ts +++ b/src/plugins/install.ts @@ -1,13 +1,12 @@ import fs from "node:fs/promises"; import path from "node:path"; import { MANIFEST_KEY } from "../compat/legacy-names.js"; +import { fileExists, readJsonFile, resolveArchiveKind } from "../infra/archive.js"; +import { resolveExistingInstallPath, withExtractedArchiveRoot } from "../infra/install-flow.js"; import { - extractArchive, - fileExists, - readJsonFile, - resolveArchiveKind, - resolvePackedRootDir, -} from "../infra/archive.js"; + resolveInstallModeOptions, + resolveTimedInstallModeOptions, +} from "../infra/install-mode-options.js"; import { installPackageDir } from "../infra/install-package-dir.js"; import { resolveSafeInstallDir, @@ -18,9 +17,11 @@ import { type NpmIntegrityDrift, type NpmSpecResolution, resolveArchiveSourcePath, - withTempDir, } from "../infra/install-source-utils.js"; -import { installFromNpmSpecArchive } from "../infra/npm-pack-install.js"; +import { + finalizeNpmSpecArchiveInstall, + installFromNpmSpecArchiveWithInstaller, +} from "../infra/npm-pack-install.js"; import { validateRegistryNpmSpec } from "../infra/npm-registry-spec.js"; import { extensionUsesSkippedScannerPath, isPathInside } from "../security/scan-paths.js"; import * as skillScanner from "../security/skill-scanner.js"; @@ -87,35 +88,6 @@ async function ensureOpenClawExtensions(manifest: PackageManifest) { return list; } -function resolvePluginInstallModeOptions(params: { - logger?: PluginInstallLogger; - mode?: "install" | "update"; - dryRun?: boolean; -}): { logger: PluginInstallLogger; mode: "install" | "update"; dryRun: boolean } { - return { - logger: params.logger ?? defaultLogger, - mode: params.mode ?? "install", - dryRun: params.dryRun ?? false, - }; -} - -function resolveTimedPluginInstallModeOptions(params: { - logger?: PluginInstallLogger; - timeoutMs?: number; - mode?: "install" | "update"; - dryRun?: boolean; -}): { - logger: PluginInstallLogger; - timeoutMs: number; - mode: "install" | "update"; - dryRun: boolean; -} { - return { - ...resolvePluginInstallModeOptions(params), - timeoutMs: params.timeoutMs ?? 120_000, - }; -} - function buildFileInstallResult(pluginId: string, targetFile: string): InstallPluginResult { return { ok: true, @@ -155,7 +127,7 @@ async function installPluginFromPackageDir(params: { dryRun?: boolean; expectedPluginId?: string; }): Promise { - const { logger, timeoutMs, mode, dryRun } = resolveTimedPluginInstallModeOptions(params); + const { logger, timeoutMs, mode, dryRun } = resolveTimedInstallModeOptions(params, defaultLogger); const manifestPath = path.join(params.packageDir, "package.json"); if (!(await fileExists(manifestPath))) { @@ -318,38 +290,21 @@ export async function installPluginFromArchive(params: { } const archivePath = archivePathResult.path; - return await withTempDir("openclaw-plugin-", async (tmpDir) => { - const extractDir = path.join(tmpDir, "extract"); - await fs.mkdir(extractDir, { recursive: true }); - - logger.info?.(`Extracting ${archivePath}…`); - try { - await extractArchive({ - archivePath, - destDir: extractDir, + return await withExtractedArchiveRoot({ + archivePath, + tempDirPrefix: "openclaw-plugin-", + timeoutMs, + logger, + onExtracted: async (packageDir) => + await installPluginFromPackageDir({ + packageDir, + extensionsDir: params.extensionsDir, timeoutMs, logger, - }); - } catch (err) { - return { ok: false, error: `failed to extract archive: ${String(err)}` }; - } - - let packageDir = ""; - try { - packageDir = await resolvePackedRootDir(extractDir); - } catch (err) { - return { ok: false, error: String(err) }; - } - - return await installPluginFromPackageDir({ - packageDir, - extensionsDir: params.extensionsDir, - timeoutMs, - logger, - mode, - dryRun: params.dryRun, - expectedPluginId: params.expectedPluginId, - }); + mode, + dryRun: params.dryRun, + expectedPluginId: params.expectedPluginId, + }), }); } @@ -389,7 +344,7 @@ export async function installPluginFromFile(params: { mode?: "install" | "update"; dryRun?: boolean; }): Promise { - const { logger, mode, dryRun } = resolvePluginInstallModeOptions(params); + const { logger, mode, dryRun } = resolveInstallModeOptions(params, defaultLogger); const filePath = resolveUserPath(params.filePath); if (!(await fileExists(filePath))) { @@ -434,7 +389,7 @@ export async function installPluginFromNpmSpec(params: { expectedIntegrity?: string; onIntegrityDrift?: (params: PluginNpmIntegrityDriftParams) => boolean | Promise; }): Promise { - const { logger, timeoutMs, mode, dryRun } = resolveTimedPluginInstallModeOptions(params); + const { logger, timeoutMs, mode, dryRun } = resolveTimedInstallModeOptions(params, defaultLogger); const expectedPluginId = params.expectedPluginId; const spec = params.spec.trim(); const specError = validateRegistryNpmSpec(spec); @@ -443,7 +398,7 @@ export async function installPluginFromNpmSpec(params: { } logger.info?.(`Downloading ${spec}…`); - const flowResult = await installFromNpmSpecArchive({ + const flowResult = await installFromNpmSpecArchiveWithInstaller({ tempDirPrefix: "openclaw-npm-pack-", spec, timeoutMs, @@ -452,28 +407,17 @@ export async function installPluginFromNpmSpec(params: { warn: (message) => { logger.warn?.(message); }, - installFromArchive: async ({ archivePath }) => - await installPluginFromArchive({ - archivePath, - extensionsDir: params.extensionsDir, - timeoutMs, - logger, - mode, - dryRun, - expectedPluginId, - }), + installFromArchive: installPluginFromArchive, + archiveInstallParams: { + extensionsDir: params.extensionsDir, + timeoutMs, + logger, + mode, + dryRun, + expectedPluginId, + }, }); - if (!flowResult.ok) { - return flowResult; - } - if (!flowResult.installResult.ok) { - return flowResult.installResult; - } - return { - ...flowResult.installResult, - npmResolution: flowResult.npmResolution, - integrityDrift: flowResult.integrityDrift, - }; + return finalizeNpmSpecArchiveInstall(flowResult); } export async function installPluginFromPath(params: { @@ -485,12 +429,12 @@ export async function installPluginFromPath(params: { dryRun?: boolean; expectedPluginId?: string; }): Promise { - const resolved = resolveUserPath(params.path); - if (!(await fileExists(resolved))) { - return { ok: false, error: `path not found: ${resolved}` }; + const pathResult = await resolveExistingInstallPath(params.path); + if (!pathResult.ok) { + return pathResult; } + const { resolvedPath: resolved, stat } = pathResult; - const stat = await fs.stat(resolved); if (stat.isDirectory()) { return await installPluginFromDir({ dirPath: resolved, diff --git a/src/slack/monitor/media.test.ts b/src/slack/monitor/media.test.ts index b3d0bcb51b7..eeeb5c88db7 100644 --- a/src/slack/monitor/media.test.ts +++ b/src/slack/monitor/media.test.ts @@ -2,6 +2,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import * as ssrf from "../../infra/net/ssrf.js"; import type { SavedMedia } from "../../media/store.js"; import * as mediaStore from "../../media/store.js"; +import { mockPinnedHostnameResolution } from "../../test-helpers/ssrf.js"; import { type FetchMock, withFetchPreconnect } from "../../test-utils/fetch-mock.js"; import { fetchWithSlackAuth, @@ -173,15 +174,7 @@ describe("resolveSlackMedia", () => { beforeEach(() => { mockFetch = vi.fn(); globalThis.fetch = withFetchPreconnect(mockFetch); - vi.spyOn(ssrf, "resolvePinnedHostname").mockImplementation(async (hostname) => { - const normalized = hostname.trim().toLowerCase().replace(/\.$/, ""); - const addresses = ["93.184.216.34"]; - return { - hostname: normalized, - addresses, - lookup: ssrf.createPinnedLookup({ hostname: normalized, addresses }), - }; - }); + mockPinnedHostnameResolution(); }); afterEach(() => { diff --git a/src/telegram/bot-message-dispatch.test.ts b/src/telegram/bot-message-dispatch.test.ts index 231fcbf4c49..5e080e90eb3 100644 --- a/src/telegram/bot-message-dispatch.test.ts +++ b/src/telegram/bot-message-dispatch.test.ts @@ -404,6 +404,37 @@ describe("dispatchTelegramMessage draft streaming", () => { expect(draftStream.stop).toHaveBeenCalled(); }); + it("keeps streamed preview visible when final text regresses after a tool warning", async () => { + const draftStream = createDraftStream(999); + createTelegramDraftStream.mockReturnValue(draftStream); + dispatchReplyWithBufferedBlockDispatcher.mockImplementation( + async ({ dispatcherOptions, replyOptions }) => { + await replyOptions?.onPartialReply?.({ text: "Recovered final answer." }); + await dispatcherOptions.deliver( + { text: "⚠️ Recovered tool error details", isError: true }, + { kind: "tool" }, + ); + await dispatcherOptions.deliver({ text: "Recovered final answer" }, { kind: "final" }); + return { queuedFinal: true }; + }, + ); + deliverReplies.mockResolvedValue({ delivered: true }); + + await dispatchWithContext({ context: createContext(), streamMode: "partial" }); + + // Regressive final ("answer." -> "answer") should keep the preview instead + // of clearing it and leaving only the tool warning visible. + expect(editMessageTelegram).not.toHaveBeenCalled(); + expect(deliverReplies).toHaveBeenCalledTimes(1); + expect(deliverReplies).toHaveBeenCalledWith( + expect.objectContaining({ + replies: [expect.objectContaining({ text: "⚠️ Recovered tool error details" })], + }), + ); + expect(draftStream.clear).not.toHaveBeenCalled(); + expect(draftStream.stop).toHaveBeenCalled(); + }); + it("falls back to normal delivery when preview final is too long to edit", async () => { const draftStream = createDraftStream(999); createTelegramDraftStream.mockReturnValue(draftStream); diff --git a/src/test-helpers/ssrf.ts b/src/test-helpers/ssrf.ts new file mode 100644 index 00000000000..1669f45bc6d --- /dev/null +++ b/src/test-helpers/ssrf.ts @@ -0,0 +1,14 @@ +import { vi } from "vitest"; +import * as ssrf from "../infra/net/ssrf.js"; + +export function mockPinnedHostnameResolution(addresses: string[] = ["93.184.216.34"]) { + return vi.spyOn(ssrf, "resolvePinnedHostname").mockImplementation(async (hostname) => { + const normalized = hostname.trim().toLowerCase().replace(/\.$/, ""); + const pinnedAddresses = [...addresses]; + return { + hostname: normalized, + addresses: pinnedAddresses, + lookup: ssrf.createPinnedLookup({ hostname: normalized, addresses: pinnedAddresses }), + }; + }); +} diff --git a/src/test-utils/channel-plugins.ts b/src/test-utils/channel-plugins.ts index ab74e932cfb..4de1680339b 100644 --- a/src/test-utils/channel-plugins.ts +++ b/src/test-utils/channel-plugins.ts @@ -51,6 +51,27 @@ export const createChannelTestPluginBase = (params: { }, }); +export const createMSTeamsTestPluginBase = (): Pick< + ChannelPlugin, + "id" | "meta" | "capabilities" | "config" +> => { + const base = createChannelTestPluginBase({ + id: "msteams", + label: "Microsoft Teams", + docsPath: "/channels/msteams", + config: { listAccountIds: () => [], resolveAccount: () => ({}) }, + }); + return { + ...base, + meta: { + ...base.meta, + selectionLabel: "Microsoft Teams (Bot Framework)", + blurb: "Bot Framework; enterprise support.", + aliases: ["teams"], + }, + }; +}; + export const createOutboundTestPlugin = (params: { id: ChannelId; outbound: ChannelOutboundAdapter; diff --git a/src/test-utils/npm-spec-install-test-helpers.ts b/src/test-utils/npm-spec-install-test-helpers.ts new file mode 100644 index 00000000000..23c06afe44b --- /dev/null +++ b/src/test-utils/npm-spec-install-test-helpers.ts @@ -0,0 +1,94 @@ +import { expect } from "vitest"; +import type { SpawnResult } from "../process/exec.js"; +import { expectSingleNpmInstallIgnoreScriptsCall } from "./exec-assertions.js"; + +export type InstallResultLike = { + ok: boolean; + error?: string; +}; + +export type NpmPackMetadata = { + id: string; + name: string; + version: string; + filename: string; + integrity: string; + shasum: string; +}; + +export function createSuccessfulSpawnResult(stdout = ""): SpawnResult { + return { + code: 0, + stdout, + stderr: "", + signal: null, + killed: false, + termination: "exit", + }; +} + +export async function expectUnsupportedNpmSpec( + install: (spec: string) => Promise, + spec = "github:evil/evil", +) { + const result = await install(spec); + expect(result.ok).toBe(false); + if (result.ok) { + return; + } + expect(result.error).toContain("unsupported npm spec"); +} + +export function mockNpmPackMetadataResult( + run: { mockResolvedValue: (value: SpawnResult) => unknown }, + metadata: NpmPackMetadata, +) { + run.mockResolvedValue(createSuccessfulSpawnResult(JSON.stringify([metadata]))); +} + +export function expectIntegrityDriftRejected(params: { + onIntegrityDrift: (...args: unknown[]) => unknown; + result: InstallResultLike; + expectedIntegrity: string; + actualIntegrity: string; +}) { + expect(params.onIntegrityDrift).toHaveBeenCalledWith( + expect.objectContaining({ + expectedIntegrity: params.expectedIntegrity, + actualIntegrity: params.actualIntegrity, + }), + ); + expect(params.result.ok).toBe(false); + if (params.result.ok) { + return; + } + expect(params.result.error).toContain("integrity drift"); +} + +export async function expectInstallUsesIgnoreScripts(params: { + run: { + mockResolvedValue: (value: SpawnResult) => unknown; + mock: { calls: unknown[][] }; + }; + install: () => Promise< + | { + ok: true; + targetDir: string; + } + | { + ok: false; + error?: string; + } + >; +}) { + params.run.mockResolvedValue(createSuccessfulSpawnResult()); + const result = await params.install(); + expect(result.ok).toBe(true); + if (!result.ok) { + return; + } + expectSingleNpmInstallIgnoreScriptsCall({ + calls: params.run.mock.calls as Array<[unknown, { cwd?: string } | undefined]>, + expectedCwd: result.targetDir, + }); +} diff --git a/src/utils/message-channel.test.ts b/src/utils/message-channel.test.ts index 9d8ca19356d..98bd4fffce2 100644 --- a/src/utils/message-channel.test.ts +++ b/src/utils/message-channel.test.ts @@ -1,43 +1,13 @@ import { afterEach, beforeEach, describe, expect, it } from "vitest"; import type { ChannelPlugin } from "../channels/plugins/types.js"; -import type { PluginRegistry } from "../plugins/registry.js"; import { setActivePluginRegistry } from "../plugins/runtime.js"; +import { createMSTeamsTestPluginBase, createTestRegistry } from "../test-utils/channel-plugins.js"; import { resolveGatewayMessageChannel } from "./message-channel.js"; -const createRegistry = (channels: PluginRegistry["channels"]): PluginRegistry => ({ - plugins: [], - tools: [], - hooks: [], - typedHooks: [], - channels, - commands: [], - providers: [], - gatewayHandlers: {}, - httpHandlers: [], - httpRoutes: [], - cliRegistrars: [], - services: [], - diagnostics: [], -}); - -const emptyRegistry = createRegistry([]); - -const msteamsPlugin = { - id: "msteams", - meta: { - id: "msteams", - label: "Microsoft Teams", - selectionLabel: "Microsoft Teams (Bot Framework)", - docsPath: "/channels/msteams", - blurb: "Bot Framework; enterprise support.", - aliases: ["teams"], - }, - capabilities: { chatTypes: ["direct"] }, - config: { - listAccountIds: () => [], - resolveAccount: () => ({}), - }, -} satisfies ChannelPlugin; +const emptyRegistry = createTestRegistry([]); +const msteamsPlugin: ChannelPlugin = { + ...createMSTeamsTestPluginBase(), +}; describe("message-channel", () => { beforeEach(() => { @@ -57,7 +27,7 @@ describe("message-channel", () => { it("normalizes plugin aliases when registered", () => { setActivePluginRegistry( - createRegistry([{ pluginId: "msteams", plugin: msteamsPlugin, source: "test" }]), + createTestRegistry([{ pluginId: "msteams", plugin: msteamsPlugin, source: "test" }]), ); expect(resolveGatewayMessageChannel("teams")).toBe("msteams"); }); diff --git a/src/web/media.test.ts b/src/web/media.test.ts index 605f0dad5a0..4bfcc7fddb1 100644 --- a/src/web/media.test.ts +++ b/src/web/media.test.ts @@ -5,9 +5,9 @@ import sharp from "sharp"; import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest"; import { resolveStateDir } from "../config/paths.js"; import { sendVoiceMessageDiscord } from "../discord/send.js"; -import * as ssrf from "../infra/net/ssrf.js"; import { resolvePreferredOpenClawTmpDir } from "../infra/tmp-openclaw-dir.js"; import { optimizeImageToPng } from "../media/image-ops.js"; +import { mockPinnedHostnameResolution } from "../test-helpers/ssrf.js"; import { captureEnv } from "../test-utils/env.js"; import { LocalMediaAccessError, @@ -126,15 +126,7 @@ describe("web media loading", () => { }); beforeAll(() => { - vi.spyOn(ssrf, "resolvePinnedHostname").mockImplementation(async (hostname) => { - const normalized = hostname.trim().toLowerCase().replace(/\.$/, ""); - const addresses = ["93.184.216.34"]; - return { - hostname: normalized, - addresses, - lookup: ssrf.createPinnedLookup({ hostname: normalized, addresses }), - }; - }); + mockPinnedHostnameResolution(); }); it("strips MEDIA: prefix before reading local file (including whitespace variants)", async () => { @@ -240,6 +232,18 @@ describe("web media loading", () => { fetchMock.mockRestore(); }); + it("keeps raw mode when options object sets optimizeImages true", async () => { + const { buffer, file } = await createLargeTestJpeg(); + const cap = Math.max(1, Math.floor(buffer.length * 0.8)); + + await expect( + loadWebMediaRaw(file, { + maxBytes: cap, + optimizeImages: true, + }), + ).rejects.toThrow(/Media exceeds/i); + }); + it("uses content-disposition filename when available", async () => { const fetchMock = vi.spyOn(globalThis, "fetch").mockResolvedValueOnce({ ok: true, diff --git a/src/web/media.ts b/src/web/media.ts index d07e5269050..cccd88e71f3 100644 --- a/src/web/media.ts +++ b/src/web/media.ts @@ -34,6 +34,27 @@ type WebMediaOptions = { readFile?: (filePath: string) => Promise; }; +function resolveWebMediaOptions(params: { + maxBytesOrOptions?: number | WebMediaOptions; + options?: { ssrfPolicy?: SsrFPolicy; localRoots?: readonly string[] | "any" }; + optimizeImages: boolean; +}): WebMediaOptions { + if (typeof params.maxBytesOrOptions === "number" || params.maxBytesOrOptions === undefined) { + return { + maxBytes: params.maxBytesOrOptions, + optimizeImages: params.optimizeImages, + ssrfPolicy: params.options?.ssrfPolicy, + localRoots: params.options?.localRoots, + }; + } + return { + ...params.maxBytesOrOptions, + optimizeImages: params.optimizeImages + ? (params.maxBytesOrOptions.optimizeImages ?? true) + : false, + }; +} + export type LocalMediaAccessErrorCode = | "path-not-allowed" | "invalid-root" @@ -385,18 +406,10 @@ export async function loadWebMedia( maxBytesOrOptions?: number | WebMediaOptions, options?: { ssrfPolicy?: SsrFPolicy; localRoots?: readonly string[] | "any" }, ): Promise { - if (typeof maxBytesOrOptions === "number" || maxBytesOrOptions === undefined) { - return await loadWebMediaInternal(mediaUrl, { - maxBytes: maxBytesOrOptions, - optimizeImages: true, - ssrfPolicy: options?.ssrfPolicy, - localRoots: options?.localRoots, - }); - } - return await loadWebMediaInternal(mediaUrl, { - ...maxBytesOrOptions, - optimizeImages: maxBytesOrOptions.optimizeImages ?? true, - }); + return await loadWebMediaInternal( + mediaUrl, + resolveWebMediaOptions({ maxBytesOrOptions, options, optimizeImages: true }), + ); } export async function loadWebMediaRaw( @@ -404,18 +417,10 @@ export async function loadWebMediaRaw( maxBytesOrOptions?: number | WebMediaOptions, options?: { ssrfPolicy?: SsrFPolicy; localRoots?: readonly string[] | "any" }, ): Promise { - if (typeof maxBytesOrOptions === "number" || maxBytesOrOptions === undefined) { - return await loadWebMediaInternal(mediaUrl, { - maxBytes: maxBytesOrOptions, - optimizeImages: false, - ssrfPolicy: options?.ssrfPolicy, - localRoots: options?.localRoots, - }); - } - return await loadWebMediaInternal(mediaUrl, { - ...maxBytesOrOptions, - optimizeImages: false, - }); + return await loadWebMediaInternal( + mediaUrl, + resolveWebMediaOptions({ maxBytesOrOptions, options, optimizeImages: false }), + ); } export async function optimizeImageToJpeg( diff --git a/ui/src/i18n/locales/en.ts b/ui/src/i18n/locales/en.ts index e99399f0e14..1f733ed6605 100644 --- a/ui/src/i18n/locales/en.ts +++ b/ui/src/i18n/locales/en.ts @@ -98,6 +98,11 @@ export const en: TranslationMap = { failed: "Auth failed. Re-copy a tokenized URL with {command}, or update the token, then click Connect.", }, + pairing: { + hint: "This device needs pairing approval from the gateway host.", + mobileHint: + "On mobile? Copy the full URL (including #token=...) from openclaw dashboard --no-open on your desktop.", + }, insecure: { hint: "This page is HTTP, so the browser blocks device identity. Use HTTPS (Tailscale Serve) or open {url} on the gateway host.", stayHttp: "If you must stay on HTTP, set {config} (token-only).", diff --git a/ui/src/i18n/locales/pt-BR.ts b/ui/src/i18n/locales/pt-BR.ts index 13b7ab92268..c13b9786a06 100644 --- a/ui/src/i18n/locales/pt-BR.ts +++ b/ui/src/i18n/locales/pt-BR.ts @@ -99,6 +99,11 @@ export const pt_BR: TranslationMap = { failed: "Falha na autenticação. Recopie uma URL com token usando {command}, ou atualize o token e clique em Conectar.", }, + pairing: { + hint: "Este dispositivo precisa de aprovação de pareamento do host do gateway.", + mobileHint: + "No celular? Copie a URL completa (incluindo #token=...) executando openclaw dashboard --no-open no desktop.", + }, insecure: { hint: "Esta página é HTTP, então o navegador bloqueia a identidade do dispositivo. Use HTTPS (Tailscale Serve) ou abra {url} no host do gateway.", stayHttp: "Se você precisar permanecer em HTTP, defina {config} (apenas token).", diff --git a/ui/src/i18n/locales/zh-CN.ts b/ui/src/i18n/locales/zh-CN.ts index f7e9a99d2e0..df2a9503db8 100644 --- a/ui/src/i18n/locales/zh-CN.ts +++ b/ui/src/i18n/locales/zh-CN.ts @@ -96,6 +96,11 @@ export const zh_CN: TranslationMap = { required: "此网关需要身份验证。添加令牌或密码,然后点击连接。", failed: "身份验证失败。请使用 {command} 重新复制令牌化 URL,或更新令牌,然后点击连接。", }, + pairing: { + hint: "此设备需要网关主机的配对批准。", + mobileHint: + "在手机上?从桌面运行 openclaw dashboard --no-open 复制完整 URL(包括 #token=...)。", + }, insecure: { hint: "此页面为 HTTP,因此浏览器阻止设备标识。请使用 HTTPS (Tailscale Serve) 或在网关主机上打开 {url}。", stayHttp: "如果您必须保持 HTTP,请设置 {config} (仅限令牌)。", diff --git a/ui/src/i18n/locales/zh-TW.ts b/ui/src/i18n/locales/zh-TW.ts index a64c7bf0953..b3591f19cf3 100644 --- a/ui/src/i18n/locales/zh-TW.ts +++ b/ui/src/i18n/locales/zh-TW.ts @@ -96,6 +96,11 @@ export const zh_TW: TranslationMap = { required: "此網關需要身份驗證。添加令牌或密碼,然後點擊連接。", failed: "身份驗證失敗。請使用 {command} 重新複製令牌化 URL,或更新令牌,然後點擊連接。", }, + pairing: { + hint: "此裝置需要閘道主機的配對批准。", + mobileHint: + "在手機上?從桌面執行 openclaw dashboard --no-open 複製完整 URL(包括 #token=...)。", + }, insecure: { hint: "此頁面為 HTTP,因此瀏覽器阻止設備標識。請使用 HTTPS (Tailscale Serve) 或在網關主機上打開 {url}。", stayHttp: "如果您必須保持 HTTP,請設置 {config} (僅限令牌)。", diff --git a/ui/src/ui/views/overview-hints.ts b/ui/src/ui/views/overview-hints.ts new file mode 100644 index 00000000000..c7ff78b9e69 --- /dev/null +++ b/ui/src/ui/views/overview-hints.ts @@ -0,0 +1,7 @@ +/** Whether the overview should show device-pairing guidance for this error. */ +export function shouldShowPairingHint(connected: boolean, lastError: string | null): boolean { + if (connected || !lastError) { + return false; + } + return lastError.toLowerCase().includes("pairing required"); +} diff --git a/ui/src/ui/views/overview.node.test.ts b/ui/src/ui/views/overview.node.test.ts new file mode 100644 index 00000000000..b8096661a3a --- /dev/null +++ b/ui/src/ui/views/overview.node.test.ts @@ -0,0 +1,28 @@ +import { describe, expect, it } from "vitest"; +import { shouldShowPairingHint } from "./overview-hints.ts"; + +describe("shouldShowPairingHint", () => { + it("returns true for 'pairing required' close reason", () => { + expect(shouldShowPairingHint(false, "disconnected (1008): pairing required")).toBe(true); + }); + + it("matches case-insensitively", () => { + expect(shouldShowPairingHint(false, "Pairing Required")).toBe(true); + }); + + it("returns false when connected", () => { + expect(shouldShowPairingHint(true, "disconnected (1008): pairing required")).toBe(false); + }); + + it("returns false when lastError is null", () => { + expect(shouldShowPairingHint(false, null)).toBe(false); + }); + + it("returns false for unrelated errors", () => { + expect(shouldShowPairingHint(false, "disconnected (1006): no reason")).toBe(false); + }); + + it("returns false for auth errors", () => { + expect(shouldShowPairingHint(false, "disconnected (4008): unauthorized")).toBe(false); + }); +}); diff --git a/ui/src/ui/views/overview.ts b/ui/src/ui/views/overview.ts index 07e07bc1b05..97b494da7f6 100644 --- a/ui/src/ui/views/overview.ts +++ b/ui/src/ui/views/overview.ts @@ -16,6 +16,7 @@ import type { import { renderOverviewAttention } from "./overview-attention.ts"; import { renderOverviewCards } from "./overview-cards.ts"; import { renderOverviewEventLog } from "./overview-event-log.ts"; +import { shouldShowPairingHint } from "./overview-hints.ts"; import { renderOverviewLogTail } from "./overview-log-tail.ts"; export type OverviewProps = { @@ -64,6 +65,34 @@ export function renderOverview(props: OverviewProps) { const authMode = snapshot?.authMode; const isTrustedProxy = authMode === "trusted-proxy"; + const pairingHint = (() => { + if (!shouldShowPairingHint(props.connected, props.lastError)) { + return null; + } + return html` +
+ ${t("overview.pairing.hint")} +
+ openclaw devices list
+ openclaw devices approve <requestId> +
+
+ ${t("overview.pairing.mobileHint")} +
+ +
+ `; + })(); + const authHint = (() => { if (props.connected || !props.lastError) { return null; @@ -293,6 +322,7 @@ export function renderOverview(props: OverviewProps) { props.lastError ? html`
${props.lastError}
+ ${pairingHint ?? ""} ${authHint ?? ""} ${insecureContextHint ?? ""}
`